lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1466121559-22363-7-git-send-email-mcgrof@kernel.org>
Date:	Thu, 16 Jun 2016 16:59:17 -0700
From:	"Luis R. Rodriguez" <mcgrof@...nel.org>
To:	ming.lei@...onical.com, akpm@...ux-foundation.org, mmarek@...e.com,
	gregkh@...uxfoundation.org, bp@...en8.de, chunkeey@...glemail.com
Cc:	linux-kernel@...r.kernel.org, markivx@...eaurora.org,
	stephen.boyd@...aro.org, zohar@...ux.vnet.ibm.com,
	broonie@...nel.org, tiwai@...e.de, johannes@...solutions.net,
	hauke@...ke-m.de, jwboyer@...oraproject.org,
	dmitry.torokhov@...il.com, dwmw2@...radead.org, jslaby@...e.com,
	torvalds@...ux-foundation.org, luto@...capital.net,
	fengguang.wu@...el.com, rpurdie@...ys.net, ki@...sung.com,
	Abhay_Salunke@...l.com, Julia.Lawall@...6.fr,
	Gilles.Muller@...6.fr, nicolas.palix@...g.fr, teg@...m.no,
	dhowells@...hat.com, keescook@...omium.org, tj@...nel.org,
	daniel.vetter@...ll.ch, corbet@....net,
	"Luis R. Rodriguez" <mcgrof@...nel.org>
Subject: [PATCH v2 6/8] Documentation/firmware_class: add sysdata API converter SmPL patch

This adds an SmPL patch to let you help you convert
drivers over from the old firmware API to the new sysdata
API. Given the amount of changes for the sync case, and the
amount of manual revision needed these patches are kept out
of scripts/coccinelle/ to annotate required developer intervention
on the convertion.

3 Coccinelle patches are provided then in Documentation/firmware_class/:

0001-convert-sysdata-sync.cocci - sync work
0002-convert-sysdata-async.cocci - for async calls
0003-convert-sysdata-generic.cocci - generic work

To use Coccinelle to help convert your driver over using these helpers in
one shot you can use the provided script as follows:

./Documentation/firmware_class/convert-sysdata.sh path-to-driver/

Contrary to the scripts/coccinelle tradition to send changes to stdout,
this uses the spatch --in-place option to make the required changes
in your git tree, to see the resulting effects you can use 'git diff'.
You should revise the code yourself, and ensure that it is correct,
compile it and run time test it before submitting a patch to transform
it using this series of SmPL patches. Be aware that this currently only
address local variable uses of the firmware, so if you stuff your
struct firmware on a data structure this is not safe to use yet.

A few release notes about these Coccinelle SmPL transformation rules used:

a) If you see if (__true__) in your changes, the sync rule add_cb_sync
   was unable to complete the work, you should review this and do the
   change yourself

b) The add_desc_sync rule deals with two cases, one where the
   const struct sysdata_file_desc sysdata_desc sysdata_desc is
   properly initialized, and the other where its initialization
   occurs later in code. Since the uninitialized case can be identified
   through a compile error, to help simplify this rule and help with
   the transition the rule is kept, developers however should review
   the changes and ensure the descriptor is properly initialized.

c) GFP_KERNEL is assumed, through an easy change the script can enable use of
   ATOMIC cases as well, however these haven't been reviewed yet.

d) FW_ACTION_NOHOTPLUG is not handled -- these require the usermode helper
   and the sysdata API purposely ignores this.

e) Coccinelle 1.0.5 will produce "return0 instead of return 0" in a few
   cases, this will be fixed in a future Coccinelle release.

f) Coccinelle 1.0.5 is required

g) The output will complain about:
    warning: line 501: should sysdata be a metavariable?
   This can safely be ignored for now.

If you use this SmPL patch to help transform your driver please use the tag:

   Generated-by: Coccinelle SmPL

Signed-off-by: Luis R. Rodriguez <mcgrof@...nel.org>
---
 .../firmware_class/0001-convert-sysdata-sync.cocci | 1154 ++++++++++++++++++++
 .../0002-convert-sysdata-async.cocci               |  259 +++++
 .../0003-convert-sysdata-generic.cocci             |   43 +
 Documentation/firmware_class/convert-sysdata.sh    |   13 +
 Documentation/firmware_class/system_data.txt       |   49 +
 5 files changed, 1518 insertions(+)
 create mode 100644 Documentation/firmware_class/0001-convert-sysdata-sync.cocci
 create mode 100644 Documentation/firmware_class/0002-convert-sysdata-async.cocci
 create mode 100644 Documentation/firmware_class/0003-convert-sysdata-generic.cocci
 create mode 100755 Documentation/firmware_class/convert-sysdata.sh

diff --git a/Documentation/firmware_class/0001-convert-sysdata-sync.cocci b/Documentation/firmware_class/0001-convert-sysdata-sync.cocci
new file mode 100644
index 000000000000..9fe607436af6
--- /dev/null
+++ b/Documentation/firmware_class/0001-convert-sysdata-sync.cocci
@@ -0,0 +1,1154 @@
+// Copyright: (C) 2016 Julia Lawall, Inria/LIP6.  GPLv2
+//
+// Options: --allow-inconsistent-paths --in-place  --force-diff -D index="" -D sysdata=sysdata
+// Requires: 1.0.5
+//
+// Identify the functions that we can treat.
+// Request_firmware must be called in the main execution path, not under an if
+// Unless all if branches call request_firmware
+// This is enforced by the ...s and their lack of when any annotation
+// This requires the first argument of request_firmware to have the form &e
+// There are a couple of calls in the kernel where this property does not hold
+
+virtual patch
+
+// identifies as function as a combination of its name and parameter list,
+// in hopes that this is unique
+
+@...tialize:ocaml@
+@@
+
+let redo index =
+  let it = new iteration() in
+  let file = List.hd (Coccilib.files()) in
+  it#set_files [file];
+  let index = if index = "" then 1 else (1+(int_of_string index)) in
+  it#add_virtual_identifier Index (string_of_int index);
+  it#add_virtual_identifier Sysdata (Printf.sprintf "sysdata%d" index);
+  it#register()
+
+@...sts@
+expression x;
+@@
+
+request_firmware(...)
+<...
+- return
++ SRETURN_(
+x
++)
+ ;
+...>
+
+@...erted@
+position p;
+@@
+
+request_firmware@p(...) == 0
+
+@...rt@
+identifier f, ret, ret1;
+local idexpression fw;
+expression name, dev;
+fresh identifier sync_found_cb = f ## "_found_cb";
+fresh identifier reqtype = f ## "_req";
+fresh identifier sreq = "sreq" ## virtual.index;
+fresh identifier context = "context" ## virtual.index;
+statement S;
+position p1 != inverted.p;
+parameter list ps;
+@@
+
+int f(ps) {
++struct reqtype { void *nothing; };
++struct reqtype sreq;
++const struct sysdata_file_desc sysdata_desc = {
++	SYSDATA_DEFAULT_SYNC(sync_found_cb, &sreq),
++};
+... when != request_firmware(...)
+(
+- ret = request_firmware@p1(&fw, name, dev);
++ ret = sysdata_file_request(name, &sysdata_desc, dev);
+if (<+...ret...+>) S
++ if(__true__) {
++ struct reqtype *sreq = context;
+  ... when any
++ return ret1;
++}
+SRETURN_(
+- ret1
++ ret
+  );
+|
+if (<+...
+-        request_firmware@p1(&fw, name, dev)
++        sysdata_file_request(name, &sysdata_desc, dev)
+    ...+>) S
++ if(__true__) {
++ struct reqtype *sreq = context;
+  ... when any
+      when != request_firmware(...)
++ return ret1;
++}
+SRETURN_(
+- ret1
++ 0
+  );
+)
+}
+
+@...oable depends on start@
+@@
+request_firmware(...)
+
+@...ipt:ocaml depends on redoable@
+index << virtual.index;
+@@
+redo index
+
+// Rename firmware structure and type
+@...sts@
+identifier start.f,virtual.sysdata;
+local idexpression start.fw;
+symbol __true__;
+parameter list start.ps;
+@@
+
+f (ps) {
+<...
+ if(__true__) {
+<...
+- fw
++ sysdata
+...>
+}
+...>
+}
+
+@@
+identifier start.f, i;
+parameter list start.ps;
+@@
+
+f (ps) {
+<...
+  struct
+- firmware
++ sysdata_file
+  i;
+...>
+}
+
+@...ends on start@
+identifier l,l1;
+@@
+
+if (...) {
+   ...
+   goto l;
+}
+... when != if (...) { ... goto l1; }
+if(__true__) {
+  ...
++ if (__false__) {
+l:
+...
++}
+}
+
+// Propagate renames in hopes of reducing the number of variables that
+// have to be stored in the context structure
+// Needs to come first because the propagated thing is not a decl, so decls
+// have to be added before it.
+@...ends on start@
+identifier b, start.reqtype, start.sreq, context;
+expression x,e1,e2,a;
+@@
+
+x = a->b;
+...  when != x = e1
+     when != a = e2
+if(__true__) {
+    struct reqtype *sreq = context;
+++  x = a->b;
+    ... when exists
+    x
+    ... when any
+}
+
+// Push downwards unused initializers as well as calls on parameter (risky)
+// requires if on ehc
+
+@@
+identifier start.f, x, start.reqtype, start.sreq, context;
+constant C;
+type T;
+symbol __false__;
+parameter list start.ps;
+@@
+
+f (ps) {
+ ... when any
+     when strict
+-T x = C;
+ ... when != x
+     when any
+ if(__true__) {
+    struct reqtype *sreq = context;
+++  T x = C;
+    <+...x...+>
+    if (__false__) { ... when != x
+    }
+ }
+... when != x
+    when any
+}
+
+@@ // structs are like constants
+identifier start.f, x, i, start.reqtype, start.sreq, context;
+symbol __false__;
+parameter list start.ps;
+@@
+
+f (ps) {
+ ... when any
+     when strict
+-struct i x;
+ ... when != x
+     when any
+ if(__true__) {
+    struct reqtype *sreq = context;
+++  struct i x;
+    <+...x...+>
+    if (__false__) { ... when != x
+    }
+ }
+... when != x
+    when any
+}
+
+@@
+identifier start.f, h, x, i, j, start.reqtype, start.sreq, context;
+expression e,e1;
+type T,T1;
+@@
+
+f (...,T1 i,...) {
+ ... when any
+     when strict
+-T x = \(i->@e j\|h(i)@e\);
+ ... when != x
+     when != i = e1
+     when any
+ if(__true__) {
+    struct reqtype *sreq = context;
+++  T x = e;
+    <+...x...+>
+    if (__false__) { ... when != x
+    }
+ }
+... when != x
+    when any
+}
+
+@...alse@
+identifier start.f;
+parameter list start.ps;
+statement S;
+@@
+
+f(ps) {
+  ... when != if (__false__) S
+}
+
+@...ends on nofalse@
+identifier start.f, x, start.reqtype, start.sreq, context;
+constant C;
+type T;
+parameter list start.ps;
+@@
+
+f (ps) {
+ ... when any
+     when strict
+-T x = C;
+ ... when != x
+     when any
+ if(__true__) {
+    struct reqtype *sreq = context;
+++  T x = C;
+    ... when exists
+    x
+    ... when any
+ }
+... when != x
+    when any
+}
+
+@...ends on nofalse@ // structs are like constants
+identifier start.f, x, i != start.reqtype, start.reqtype, start.sreq, context;
+parameter list start.ps;
+@@
+
+f (ps) {
+ ... when any
+     when strict
+-struct i x;
+ ... when != x
+     when any
+ if(__true__) {
+    struct reqtype *sreq = context;
+++  struct i x;
+    ... when exists
+    x
+    ... when any
+ }
+... when != x
+    when any
+}
+
+@...ends on nofalse@
+identifier start.f, h, x, i, j, start.reqtype, start.sreq, context;
+expression e,e1;
+type T,T1;
+@@
+
+f (...,T1 i,...) {
+ ... when any
+     when strict
+-T x = \(i->@e j\|h(i)@e\);
+ ... when != x
+     when != i = e1
+     when any
+ if(__true__) {
+    struct reqtype *sreq = context;
+++  T x = e;
+    ... when exists
+    x
+    ... when any
+ }
+... when != x
+    when any
+}
+
+// Initialize return vaue if needed
+
+@...ret depends on start exists@
+identifier ret,r;
+type T;
+expression e1,e2;
+statement S;
+position p;
+@@
+
+T ret;
+...
+ret = sysdata_file_request(...);
+if (...) S
+if@p(__true__) {
+  ... when != ret = e1
+      when != T ret;
+(
+  ret = e2
+|
+  ret@r
+)
+  ... when any
+}
+
+@...ends on start exists@
+identifier updret.r, start.reqtype, start.sreq, context;
+type updret.T;
+expression e1;
+position updret.p;
+@@
+
+if@p(__true__) {
+  struct reqtype *sreq = context;
+++ T r = 0;
+  ... when != r = e1
+      when != T r;
+  r
+  ... when any
+}
+
+
+// Duplicates decl
+@@
+identifier start.f, x, i, start.reqtype, start.sreq, context;
+constant C;
+type T,T1;
+expression e;
+@@
+
+f (...,T1 i,...) {
+ ... when any
+T x = C;
+ ... when != x = e
+     when any
+ if(__true__) {
+    struct reqtype *sreq = context;
+++  T x = C;
+    ... when exists
+    x
+    ... when any
+ }
+... when any
+}
+
+// Remove if on ehc
+
+@ehc depends on start exists@
+statement list sl;
+@@
+
+if(__true__) {
+  ... when any
+- if (__false__) {
+ sl
+-}
+ ... when any
+}
+
+@@
+expression x;
+@@
+
+- SRETURN_(
++ return
+  x
+- )
+  ;
+
+// Try to construct the context: working on the written variables
+
+@...te1 depends on start exists@
+position pa1,px1;
+type T;
+identifier i;
+expression e;
+@@
+
+if (__true__) { ... when any
+  T i@pa1@px1 = e;
+  ... when any
+}
+
+@...te2 depends on start exists@
+type T;
+T v;
+identifier i;
+expression e;
+position pa2,px2;
+@@
+
+if (__true__) { ... when any
+  v@i@pa2@px2 = e
+  ... when any
+}
+
+@...te3 depends on start exists@
+type T;
+T v;
+identifier i,g;
+position pa3,px3;
+@@
+
+if (__true__) { ... when any
+  g(...,&v@i@pa3@px3,...)
+  ... when any
+}
+
+@...ten@
+identifier i;
+position any write1.pa1;
+position any write2.pa2;
+position any write3.pa3;
+@@
+
+(
+i@pa1
+|
+i@pa2
+|
+i@pa3
+)
+
+// var is read on some path before it was written
+@...d depends on start exists@
+local idexpression v;
+identifier virtual.sysdata;
+identifier writen.i;
+expression e,e1;
+position p != {write1.px1,write2.px2,write3.px3};
+position any write1.pa1;
+position any write2.pa2;
+position any write3.pa3;
+statement S;
+type T;
+@@
+
+if (__true__) {
+  ... when any
+      when != T i@pa1 = e1;
+      when != i@pa2
+      when != i@pa3
+(
+  for (<+...i = e...+>; ...; ...) S
+|
+  sizeof(<+...i...+>) // not a reference
+|
+  sysdata
+|
+  v@i@p
+)
+  ... when any
+}
+
+// other occurrences of vars that were somewhere first read
+@...stread depends on start exists@
+local idexpression read.v;
+identifier writen.i;
+position p;
+@@
+
+if (__true__) {
+  <...
+  v@i@p
+  ...>
+}
+
+@r depends on start exists@
+type T;
+local idexpression T v;
+identifier writen.i,reqtype,start.f,start.sreq, context;
+position p != firstread.p;
+parameter list start.ps;
+@@
+
+f(ps) {
+...
+if (__true__) {
+   struct reqtype *sreq = context;
+  ... when != T i;
+  v@i@p
+  ... when any
+}
+... when any
+}
+
+@...ends on start@
+type r.T;
+identifier writen.i,start.f,start.sreq, context;
+identifier reqtype;
+parameter list start.ps;
+@@
+
+f(ps) {
+... when any
+if (__true__) {
+   struct reqtype *sreq = context;
+++ T i;
+  ...
+}
+... when any
+}
+
+// Try to construct the context: working on the read variables
+
+@...ignfor depends on start exists@
+identifier i;
+expression e;
+position p,p1,p2;
+statement S;
+@@
+
+if (__true__) { ... when any
+  for(<+...i@p = e...+>; <+...i@.....+>; <+...i@.....+>) S
+  ... when any
+}
+
+@...ignaddr depends on start exists@
+identifier i,g;
+position p;
+@@
+
+if (__true__) { ... when any
+  g(...,&i@p,...)
+  ... when any
+}
+
+// The following overlaps with assignaddr, so has to be a separate rule
+@...ign depends on start exists@
+identifier i;
+expression e;
+position p;
+type T;
+@@
+
+if (__true__) { ... when any
+(
+  T i@p = e;
+|
+  T i@p;
+|
+  i@p = e
+)
+  ... when any
+}
+
+@...sread depends on start exists@
+position p != {assignfor.p,assignfor.p1,assignfor.p2,assignaddr.p,assign.p};
+identifier i,reqtype,start.f,start.sreq,context;
+type T,T1;
+local idexpression T v;
+parameter list start.ps;
+@@
+
+f(ps) {
+...
+if (__true__) {
+  struct reqtype *sreq = context;
+  ... when any
+      when != i // matches exp
+      when != T1 i;
+(
+  sizeof(<+...v...+>)
+|
+  v@i@p
+)
+  ... when any
+}
+... when any
+}
+
+@cc depends on start exists@
+type doesread.T,T1;
+identifier doesread.i,start.f,start.sreq,context;
+identifier reqtype, ret;
+statement S;
+parameter list start.ps;
+@@
+
+f(ps) {
+struct reqtype {
+- void *nothing;
+  ...
+++ T i;
+};
+... when any
+(
+++sreq.i = i;
+ret = sysdata_file_request(...);
+if (<+...ret...+>) S
+|
+++sreq.i = i;
+if (<+...sysdata_file_request(...)...+>) S
+)
+if (__true__) {
+  struct reqtype *sreq = context;
+++ T i = sreq->i;
+  ... when any
+      when != i // matches exp
+      when != T1 i;
+  i
+  ... when any
+}
+  ...  when any
+}
+
+// Convert arrays to pointers
+@@
+identifier cc.reqtype;
+type T;
+identifier i;
+@@
+
+struct reqtype {
+  ...
+  T
++ *
+  i
+- [...]
+  ;
+  ...
+};
+
+// Hack to deal with array types and with a weakness in the pretty printer,
+// part 2.
+@...ends on start exists@
+type T;
+identifier i,start.sreq;
+@@
+
+if (__true__) { ... when any
+  T
++ *
+  i
+- [...]
+  = sreq->i;
+  ... when any
+}
+
+@sz depends on start exists@ // vars only used in sizeof
+type T1,T;
+identifier reqtype, i, start.sreq, context;
+local idexpression T v;
+@@
+
+if (__true__) {
+  struct reqtype *sreq = context;
+++ T i;
+  ... when != T1 i;
+  sizeof(v@i)
+  ... when any
+}
+
+// Drop the context structure if it contains only one element
+
+@...pstructparam exists@
+identifier start.f,start.reqtype,x,start.sreq,context;
+type T;
+expression e;
+symbol sysdata_desc;
+@@
+
+f(...,T x,...) {
+-struct reqtype { T x; };
+-struct reqtype sreq;
+const struct sysdata_file_desc sysdata_desc = {
+	SYSDATA_DEFAULT_SYNC(e,
+-       &sreq
++       (void *)x
+        ),
+};
+...
+-sreq.x = x;
+...
+if (__true__) {
+-  struct reqtype *sreq = context;
+-  T x = sreq->x;
++  T x = context;
+   ...
+}
+... when any
+}
+
+@...pstructlocal exists@
+identifier start.f,start.reqtype,x,start.sreq,context;
+statement S;
+expression e;
+type T;
+parameter list start.ps;
+@@
+
+f(ps) {
+-struct reqtype { T x; };
+-struct reqtype sreq;
+-const struct sysdata_file_desc sysdata_desc = {
+-	SYSDATA_DEFAULT_SYNC(e, &sreq),
+-};
+... when != S
+T x = ...;
++const struct sysdata_file_desc sysdata_desc = {
++	SYSDATA_DEFAULT_SYNC(e, (void *)x),
++};
+...
+-sreq.x = x;
+...
+if (__true__) {
+-  struct reqtype *sreq = context;
+-  T x = sreq->x;
++  T x = context;
+   ...
+}
+... when any
+}
+
+@...pstruct exists@
+identifier start.f,start.reqtype,start.sreq,context;
+expression e;
+parameter list start.ps;
+@@
+
+f(ps) {
+-struct reqtype { void *nothing; };
+-struct reqtype sreq;
+const struct sysdata_file_desc sysdata_desc = {
+	SYSDATA_DEFAULT_SYNC(e,
+- &sreq
++ NULL
+  ),
+};
+...
+if (__true__) {
+-  struct reqtype *sreq = context;
+   ...
+}
+... when any
+}
+
+// Move the context structure declaration out to top level
+
+@...ends on !dropstructparam && !dropstructlocal && !dropstruct@
+identifier start.f;
+type T;
+parameter list start.ps;
+@@
+
++ T;
+
+f(ps) {
+- T;
+  ...
+}
+
+// Construct the callback function
+
+// The conjunction ( & ) is for efficiency: the first case is easy to
+// remove, while the second allows transporting the code without the braces.
+@ add_cb_sync1 depends on ehc@
+identifier start.f,virtual.sysdata;
+fresh identifier sync_found_cb = f ## "_found_cb";
+fresh identifier context = "context" ## virtual.index;
+statement S;
+statement list S1, ehc.sl;
+expression ret;
+parameter list start.ps;
+@@
+
++static int sync_found_cb(void *context, const struct sysdata_file *sysdata)
++{
++ S1
++}
+
+f (ps) {
+... when any
+(
+- if (__true__) S
+&
+if (__true__) { S1 }
+)
+-return ret;
++sl
+}
+
+@@
+expression x;
+@@
+
+- SRETURN_(x);
++ return x;
+
+@ add_cb_sync2 depends on !ehc @
+identifier start.f,virtual.sysdata;
+fresh identifier sync_found_cb = f ## "_found_cb";
+fresh identifier context = "context" ## virtual.index;
+statement S;
+statement list S1;
+expression ret;
+parameter list start.ps;
+@@
+
++static int sync_found_cb(void *context, const struct sysdata_file *sysdata)
++{
++ S1
++}
+
+f (ps) {
+... when any
+(
+- if (__true__) S
+&
+if (__true__) { S1 }
+)
+return ret;
+}
+
+// Check for a single retry
+// Not safe.  We just hope that the fw that is consistent with f, name, and dev
+// is the right one...
+
+@@
+identifier start.f,l,ret;
+expression name, dev, ret1, name1, dev1;
+local idexpression start.fw;
+statement S;
+parameter list start.ps;
+@@
+
+f(ps) { <...
+(
+ret = sysdata_file_request(name, &sysdata_desc, dev);
+if (...) {
+  ... when != goto l;
+(
+- ret1 = request_firmware(&fw, name1, dev1);
++ ret1 = sysdata_file_request(name1, &sysdata_desc, dev);
+if (<+...ret1...+>) S
+|
+if (<+...
+-        request_firmware(&fw, name1, dev1)
++        sysdata_file_request(name1, &sysdata_desc, dev)
+    ...+>) S
+)
+}
+|
+if (<+...sysdata_file_request(name, &sysdata_desc, dev)...+>) {
+  ... when != goto l;
+(
+- ret1 = request_firmware(&fw, name1, dev1);
++ ret1 = sysdata_file_request(name1, &sysdata_desc, dev);
+if (<+...ret1...+>) S
+|
+if (<+...
+-        request_firmware(&fw, name1, dev1)
++        sysdata_file_request(name1, &sysdata_desc, dev)
+    ...+>) S
+)
+}
+)
+...> }
+
+// drop trivial labels
+
+@@
+identifier start.f,out;
+local idexpression x;
+parameter list start.ps;
+@@
+
+f(ps) {
+... when != goto out;
+- x =
++ return
+   sysdata_file_request(...);
+-if (<+...x...+>) goto out;
+-out:
+-return x;
+}
+
+@@
+identifier start.f,out;
+local idexpression x;
+parameter list start.ps;
+statement S, S1;
+@@
+
+f(ps) {
+...
+ x =
+   sysdata_file_request(...);
+(
+ if (<+...x...+>)
+ {
+  if (...) S else S1
+- goto out;
+ }
+|
+ if (<+...x...+>)
+- {
+  S
+- goto out;
+- }
+)
+out:
+...
+}
+
+// drop unused labels
+
+@l1 exists@
+identifier f = {start.sync_found_cb,start.f};
+identifier l;
+position p,p1;
+@@
+
+f@p1(...) {
+  <... when any
+  l@p:
+  ...>
+}
+
+@l2@
+identifier l1.f, l1.l;
+position l1.p1;
+@@
+
+f@p1(...) {
+  ... when != goto l;
+      when strict
+}
+
+@l3 depends on l2@
+identifier l1.f, l1.l;
+position l1.p,l1.p1;
+@@
+
+f@p1(...) {
+  <...
+- l@p:
+  ...>
+}
+
+@@
+identifier start.f;
+local idexpression x;
+parameter list start.ps;
+@@
+
+f(ps) {
+...
+- x =
++ return
+   sysdata_file_request(...);
+-if (<+...x...+>) return x;
+-return x;
+...
+}
+
+@@
+identifier start.f;
+local idexpression x;
+statement S,S1;
+parameter list start.ps;
+@@
+
+f(ps) {
+...
+ x =
+   sysdata_file_request(...);
+(
+if (...) { if (...) S else S1
+- return x;
+ }
+|
+if (<+...x...+>)
+- {
+  S
+- return x;}
+)
+return x;
+}
+
+// drop any now (or previously...) unused variables
+@...ld exists@
+identifier start.f;
+type T;
+identifier x;
+constant C;
+parameter list start.ps;
+position p;
+@@
+
+f(ps) {
+  ... when any
+      when strict
+(
+ T x@p = C;
+|
+ T x@p;
+)
+  ... when any
+}
+
+@...sed@
+identifier start.f;
+identifier decld.x;
+parameter list start.ps;
+@@
+
+f(ps) {
+  ... when != x
+}
+
+@...ends on unused@
+position decld.p;
+type T;
+identifier x;
+constant C;
+@@
+
+(
+- T x@p = C;
+|
+- T x@p;
+)
+
+// common cleanup: we can't include files at the end of a cocci file yet
+// so we make this highly dependent on these set of rules.
+// Despite the care some of these changes may still be too aggressive
+// given we do not ensure the const struct firmware *data was the one
+// used on the start rule
+
+@ use_new_struct depends on start @
+identifier consumer, data;
+@@
+
+consumer(...,
+-	const struct firmware *data
++	const struct sysdata_file *data
+	,...)
+{
+...
+}
+
+@ modify_decl depends on start @
+type T;
+identifier consumer, data;
+@@
+
+T consumer(...,
+-	const struct firmware *data
++	const struct sysdata_file *data
+	,...);
+
+@ replace_struct_on_types depends on start @
+type T;
+identifier data;
+@@
+
+T {
+	...
+-	const struct firmware *data;
++	const struct sysdata_file *data;
+	...
+};
+
+@ drop_fw_release_goto depends on start @
+identifier out, some_fn;
+@@
+
+void some_fn (...) {
+<+...
+- goto out;
++ return;
+...+>
+-out:
+-release_firmware(...);
+}
+
+@ drop_fw_release_fn depends on start @
+identifier fn;
+@@
+
+-fn (...) {
+-release_firmware(...);
+-}
+
+@ drop_fw_release_fn_uses depends on start @
+identifier drop_fw_release_fn.fn;
+@@
+
+-fn(...);
+
+@ drop_fw_release_branch depends on start @
+@@
+
+-if (...)
+-release_firmware(...);
+
+@ drop_fw_release depends on start @
+@@
+
+-release_firmware(...);
diff --git a/Documentation/firmware_class/0002-convert-sysdata-async.cocci b/Documentation/firmware_class/0002-convert-sysdata-async.cocci
new file mode 100644
index 000000000000..c781bdc5da04
--- /dev/null
+++ b/Documentation/firmware_class/0002-convert-sysdata-async.cocci
@@ -0,0 +1,259 @@
+// You can use this to help convert a device driver from the old firmware
+// request_firmware_nowait() API the new flexible sysdata API for async
+// requests.
+//
+// Confidence: Medium
+//
+// Reason for low confidence:
+//
+// Copyright: (C) 2016 Luis R. Rodriguez <mcgrof@...nel.org> GPLv2.
+// Copyright: (C) 2016 Julia Lawall, Inria/LIP6.  GPLv2
+//
+// Options: --include-headers --in-place --no-show-diff
+// Requires: 1.0.5
+
+virtual patch
+
+@ async_get_t @
+expression name, dev;
+bool true;
+identifier drv_callback;
+type T;
+T *drv;
+@@
+
+(
+request_firmware_nowait(THIS_MODULE, true, name, dev, GFP_KERNEL, drv, drv_callback)
+|
+request_firmware_nowait(THIS_MODULE, 1, name, dev, GFP_KERNEL, drv, drv_callback)
+|
+request_firmware_nowait(THIS_MODULE, FW_ACTION_HOTPLUG, name, dev, GFP_KERNEL, drv, drv_callback)
+)
+
+@ find_request_async depends on async_get_t @
+expression name, dev, uevent;
+identifier drv_callback, drv, f;
+@@
+
+f (...)
+{
+<+...
+-request_firmware_nowait(THIS_MODULE, uevent, name, dev, GFP_KERNEL, drv, drv_callback)
++sysdata_file_request_async(name, &sysdata_desc, dev, &drv->sysdata_async_cookie)
+...+>
+}
+
+@ add_desc_async @
+type T;
+identifier find_request_async.f;
+identifier find_request_async.drv;
+identifier find_request_async.drv_callback;
+@@
+
+f (...)
+{
+...
+T *drv;
++const struct sysdata_file_desc sysdata_desc = {
++	SYSDATA_DEFAULT_ASYNC(drv_callback, drv),
++};
+...
+}
+
+@ add_desc_direct_async @
+type T;
+identifier find_request_async.f;
+identifier find_request_async.drv;
+identifier find_request_async.drv_callback;
+@@
+
+f (...,
+   T *drv
+   ,...)
+{
++const struct sysdata_file_desc sysdata_desc = {
++	SYSDATA_DEFAULT_ASYNC(drv_callback, drv),
++};
+...
+}
+
+@ found_callback depends on async_get_t @
+identifier find_request_async.drv_callback;
+identifier find_request_async.drv;
+identifier data, arg;
+type T1;
+@@
+
+ drv_callback(
+-const struct firmware *data,
++const struct sysdata_file *data,
+ void *arg)
+ {
+	...
+	T1 *drv = arg;
+	...
+ }
+
+@ found_completion depends on found_callback @
+identifier find_request_async.drv_callback;
+type found_callback.T1;
+T1 *drv;
+identifier cmpl;
+@@
+
+ drv_callback(...)
+ {
+	<+...
+(
+-	complete(&drv->cmpl);
+|
+-	complete_all(&drv->cmpl);
+)
+	...+>
+ }
+
+@ drop_init_completion @
+identifier find_request_async.drv;
+identifier found_completion.cmpl;
+@@
+
+-init_completion(&drv->cmpl);
+
+@ replace_completion_wait @
+identifier find_request_async.drv;
+identifier found_completion.cmpl;
+@@
+
+-wait_for_completion(&drv->cmpl);
++sysdata_synchronize_request(drv->sysdata_async_cookie);
+
+@ async_cookie exists @
+typedef async_cookie_t;
+type T;
+@@
+
+T {
+	...
+	async_cookie_t sysdata_async_cookie;
+	...
+};
+
+@ modify_drv depends on !async_cookie @
+type async_get_t.T;
+identifier found_completion.cmpl;
+@@
+
+T {
+	...
+-	struct completion cmpl;
++	async_cookie_t sysdata_async_cookie;
+	...
+};
+
+@ modify_drv2 depends on !modify_drv @
+type found_callback.T1;
+identifier found_completion.cmpl;
+@@
+
+T1 {
+	...
+-	struct completion cmpl;
++	async_cookie_t sysdata_async_cookie;
+	...
+};
+
+@ modify_drv3 depends on !(modify_drv || modify_drv2)@
+type add_desc_async.T;
+@@
+
+T {
+	...
++	async_cookie_t sysdata_async_cookie;
+};
+
+@ modify_drv_direct depends on !(modify_drv || modify_drv2 || modify_drv3) @
+type add_desc_direct_async.T;
+@@
+
+T {
+	...
++	async_cookie_t sysdata_async_cookie;
+};
+
+// common cleanup: we can't include files at the end of a cocci file yet
+// so we make this highly dependent on these set of rules.
+// Despite the care some of these changes may still be too aggressive
+// given we do not ensure the const struct firmware *data was the one
+// used on the start rule
+
+@ use_new_struct depends on add_desc_async || add_desc_direct_async @
+identifier consumer, data;
+@@
+
+consumer(...,
+-	const struct firmware *data
++	const struct sysdata_file *data
+	,...)
+{
+...
+}
+
+@ modify_decl depends on add_desc_async || add_desc_direct_async @
+type T;
+identifier consumer, data;
+@@
+
+T consumer(...,
+-	const struct firmware *data
++	const struct sysdata_file *data
+	,...);
+
+@ replace_struct_on_types depends on add_desc_async || add_desc_direct_async @
+type T;
+identifier data;
+@@
+
+T {
+	...
+-	const struct firmware *data;
++	const struct sysdata_file *data;
+	...
+};
+
+@ drop_fw_release_goto depends on add_desc_async || add_desc_direct_async @
+identifier out, some_fn;
+@@
+
+void some_fn (...) {
+<+...
+- goto out;
++ return;
+...+>
+-out:
+-release_firmware(...);
+}
+
+@ drop_fw_release_fn depends on add_desc_async || add_desc_direct_async @
+identifier fn;
+@@
+
+-fn (...) {
+-release_firmware(...);
+-}
+
+@ drop_fw_release_fn_uses depends on add_desc_async || add_desc_direct_async @
+identifier drop_fw_release_fn.fn;
+@@
+
+-fn(...);
+
+@ drop_fw_release_branch depends on add_desc_async || add_desc_direct_async @
+@@
+
+-if (...)
+-release_firmware(...);
+
+@ drop_fw_release depends on add_desc_async || add_desc_direct_async @
+@@
+
+-release_firmware(...);
diff --git a/Documentation/firmware_class/0003-convert-sysdata-generic.cocci b/Documentation/firmware_class/0003-convert-sysdata-generic.cocci
new file mode 100644
index 000000000000..de476f44a5b3
--- /dev/null
+++ b/Documentation/firmware_class/0003-convert-sysdata-generic.cocci
@@ -0,0 +1,43 @@
+// You can use this to help convert a device driver from the old firmware
+// request_firmware*() API the new flexible sysdata API, this covers the
+// generic conversions for both sync and async cases.
+//
+// Confidence: High
+//
+// Copyright: (C) 2016 Luis R. Rodriguez <mcgrof@...nel.org> GPLv2.
+// Copyright: (C) 2016 Julia Lawall, INRIA/LIP6.  GPLv2
+//
+// Options: --include-headers --in-place --no-show-diff
+// Requires: 1.0.5
+
+virtual patch
+
+@ uses_fw_api @
+@@
+
+(
+request_firmware(...)
+|
+request_firmware_nowait(...)
+)
+
+@ uses_sysdata @
+@@
+
+(
+sysdata_file_request(...)
+|
+sysdata_file_request_async(...)
+)
+
+@ replace_header depends on uses_sysdata && !uses_fw_api @
+@@
+
+-#include <linux/firmware.h>
++#include <linux/sysdata.h>
+
+@ add_header depends on uses_sysdata && uses_fw_api @
+@@
+
+#include <linux/firmware.h>
++#include <linux/sysdata.h>
diff --git a/Documentation/firmware_class/convert-sysdata.sh b/Documentation/firmware_class/convert-sysdata.sh
new file mode 100755
index 000000000000..a6a01ba4c6fa
--- /dev/null
+++ b/Documentation/firmware_class/convert-sysdata.sh
@@ -0,0 +1,13 @@
+#!/bin/bash
+
+set -e
+
+if [[ $# -ne 1 ]]; then
+	echo Usage: $0 ./path/to-driver/
+	exit
+fi
+
+for i in Documentation/firmware_class/*.cocci; do
+	export COCCI=$i
+	make coccicheck MODE=patch M=$1
+done
diff --git a/Documentation/firmware_class/system_data.txt b/Documentation/firmware_class/system_data.txt
index 53378ce4fcd0..495a5ef6a24b 100644
--- a/Documentation/firmware_class/system_data.txt
+++ b/Documentation/firmware_class/system_data.txt
@@ -86,4 +86,53 @@ Tracking development enhancements and ideas
 To help track ongoing development for firmware_class and related items to
 firmware_class refer to the kernel newbies wiki page [0].
 
+Converting old firmware API users to sysdata API
+================================================
+
+To help developers convert drivers over to the sysdata API
+a series of Coccinelle SmPL patches is provided to help with
+the transition if and when needed:
+
+Documentation/firmware_class/convert-sysdata-async.cocci - for async calls
+Documentation/firmware_class/convert-sysdata-generic.cocci - for sync calls
+Documentation/firmware_class/convert-sysdata-sync.cocci - generic work
+
+3 files are used to limit each conversion's scope and increase the speed
+for conversion. For instance the sync conversion uses the flags (--no-loops
+--no-gotos). To use Coccinelle to help convert your driver over using
+these helpers in one shot you can use the provided script as follows:
+
+./Documentation/firmware_class/convert-sysdata.sh path-to-driver/
+
+Contrary to the scripts/coccinelle tradition to send changes to stdout,
+this uses the spatch --in-place option to make the required changes
+in your git tree, to see the resulting effects you can use 'git diff'.
+You should revise the code yourself, and ensure that it is correct,
+compile it and run time test it if possible before submitting a patch
+to transform it using this SmPL patch.
+
+A few notes about these Coccinelle changes:
+
+a) If you see if (__true__) in your changes, the sync rule add_cb_sync
+   was unable to complete the work, you should review this and do the
+   change yourself
+
+b) The add_desc_sync rule deals with two cases, one where the
+   const struct sysdata_file_desc sysdata_desc sysdata_desc is properly
+   initialized, and the other where its initialization occurs later in
+   code. Since the uninitialized case can be identified through a compile
+   error, to help simplify this rule and help with the transition the rule
+   is kept, developers however should review the changes and ensure the
+   descriptor is properly initialized.
+
+c) GFP_KERNEL is assumed, an easy change the script can enable use on
+   ATOMIC cases as well, however these haven't been reviewed yet.
+
+d) FW_ACTION_NOHOTPLUG is not handled -- these require the usermode helper
+   and the sysdata API purposely ignores this.
+
+If you use this SmPL patch to transform your driver please use the tag:
+
+Generated-by: Coccinelle SmPL
+
 [0] http://kernelnewbies.org/KernelProjects/firmware-class-enhancements
-- 
2.8.2

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ