public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH] - scripts/CodingStyle_sizeof - standardize sizeof and [PATCH] drivers/infiniband - standardize sizeof(foo)
@ 2008-07-01 16:49 Joe Perches
  2008-07-01 17:57 ` Roland Dreier
  0 siblings, 1 reply; 4+ messages in thread
From: Joe Perches @ 2008-07-01 16:49 UTC (permalink / raw)
  To: LKML; +Cc: Roland Dreier, general

[-- Attachment #1: Type: text/plain, Size: 2101 bytes --]

Scripts could be used to generate CodingStyle patches
in reviewable units.

For instance, kernel source has multiple style uses of sizeof

sizeof(foo)
sizeof foo
sizeof (foo)

Perhaps Julia Lawall semantic patches might work better,
but this script might help standardize on the CodingStyle
preferred sizeof(foo).

It's an imperfect tool.  Any patch generated by a tool
(yes, I just called myself a tool..:) must be inspected
and verified before being applied.

A sample patch of drivers_infiniband.patch generated by
$ scripts/CodingStyle_sizeof drivers/infiniband
is also attached as bz2

Signed-off-by: Joe Perches <joe@perches.com>
---
 scripts/CodingStyle_sizeof |   33 +++++++++++++++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/scripts/CodingStyle_sizeof b/scripts/CodingStyle_sizeof
new file mode 100755
index 0000000..7f2444f
--- /dev/null
+++ b/scripts/CodingStyle_sizeof
@@ -0,0 +1,33 @@
+#!/bin/bash
+if [ $# -ne 1 ] ; then
+    echo "$0 takes 1 argument: directory|file"
+    exit
+fi
+if [[ ! -f $1 && ! -d $1 ]] ; then
+    echo "Directory or file: $1 not found"
+    exit
+fi
+if [ -d $1 ] ; then
+    findfile="find $1 -name \"*.[ch]\""
+else
+    findfile="ls $1"
+fi
+patchfile=$1.patch
+patchfile=${patchfile//\//_}
+git checkout $1
+for file in $(eval $findfile) ; do
+    sed -r -i -e 's/\bsizeof\s+([^\(][^ ;,]+)\s*/sizeof\(\1\)/g' $file
+    sed -r -i -e 's/\bsizeof\s+/sizeof/g' $file
+done
+git diff -p --stat $1 > $patchfile
+# fixup the generated diff
+# add space between close paren and (brace, assign, arithmetic or logical test)
+sed -r -i -e 's/^\+(.*)\)([\{\=\+\*\/\<\>\|\!\&\?\-])/\+\1\) \2/g' $patchfile
+# remove space between ') ->' added by sed above
+sed -r -i -e 's/^\+(.*)\) ->/\+\1\)->/g' $patchfile
+# fix declaration with sizeof brackets ie: char foo[sizeof("bar")];
+sed -r -i -e 's/^\+(.*)\[(.*)\((.*)\]\)/\+\1\[\2\(\3\)\]/g' $patchfile
+# fix sizeof array element ie: sizeof(foo->bar[n])
+sed -r -i -e 's/^\+(.*)sizeof\((.*)\[(.*)\)\]/\+\1sizeof\(\2\[\3\]\)/g' $patchfile
+git checkout $1
+patch -p1 < $patchfile


[-- Attachment #2: drivers_infiniband.patch.bz2 --]
[-- Type: application/x-bzip, Size: 37502 bytes --]

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [RFC PATCH] - scripts/CodingStyle_sizeof - standardize sizeof and [PATCH] drivers/infiniband - standardize sizeof(foo)
  2008-07-01 17:57 ` Roland Dreier
@ 2008-07-01 17:37   ` Joe Perches
  2008-07-01 19:21   ` [ofa-general] " Dotan Barak
  1 sibling, 0 replies; 4+ messages in thread
From: Joe Perches @ 2008-07-01 17:37 UTC (permalink / raw)
  To: Roland Dreier; +Cc: LKML, general

On Tue, 2008-07-01 at 10:57 -0700, Roland Dreier wrote:
> This seems like needless churn to me.  I personally prefer to see
> "sizeof foo" instead of "sizeof(foo)", since sizeof is not a function,
> and in any case I don't see much value in making this type of trivial
> change.

No value but standardization and ease of grep.

fyi: drivers/infiniband uses 3 different styles for sizeof.
Your preferred style is not the most frequently used.

$ grep -Pr --include=*.[ch] "\bsizeof\(" drivers/infiniband | wc -l
785
$ grep -Pr --include=*.[ch] "\bsizeof\s+\(" drivers/infiniband | wc -l
226
$ grep -Pr --include=*.[ch] "\bsizeof\s+[^\(]" drivers/infiniband | wc -l
523

cheers, Joe


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC PATCH] - scripts/CodingStyle_sizeof - standardize sizeof and [PATCH] drivers/infiniband - standardize sizeof(foo)
  2008-07-01 16:49 [RFC PATCH] - scripts/CodingStyle_sizeof - standardize sizeof and [PATCH] drivers/infiniband - standardize sizeof(foo) Joe Perches
@ 2008-07-01 17:57 ` Roland Dreier
  2008-07-01 17:37   ` Joe Perches
  2008-07-01 19:21   ` [ofa-general] " Dotan Barak
  0 siblings, 2 replies; 4+ messages in thread
From: Roland Dreier @ 2008-07-01 17:57 UTC (permalink / raw)
  To: Joe Perches; +Cc: LKML, general

This seems like needless churn to me.  I personally prefer to see
"sizeof foo" instead of "sizeof(foo)", since sizeof is not a function,
and in any case I don't see much value in making this type of trivial
change.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [ofa-general] Re: [RFC PATCH] - scripts/CodingStyle_sizeof - standardize sizeof and [PATCH] drivers/infiniband - standardize sizeof(foo)
  2008-07-01 17:57 ` Roland Dreier
  2008-07-01 17:37   ` Joe Perches
@ 2008-07-01 19:21   ` Dotan Barak
  1 sibling, 0 replies; 4+ messages in thread
From: Dotan Barak @ 2008-07-01 19:21 UTC (permalink / raw)
  To: Roland Dreier; +Cc: Joe Perches, LKML, general

Roland Dreier wrote:
> This seems like needless churn to me.  I personally prefer to see
> "sizeof foo" instead of "sizeof(foo)", since sizeof is not a function,
> and in any case I don't see much value in making this type of trivial
> change.
>   
In the IB stack i noticed all of the variations of sizeof:
sizeof foo
   or
sizeof(foo)
   (and i think that i even saw)
sizeof (foo)


IMHO, Changing all of the instances of "sizeof" from all of the 
variations to a unified one can make
the code look a little bit better ...

Dotan

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2008-07-01 18:21 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-07-01 16:49 [RFC PATCH] - scripts/CodingStyle_sizeof - standardize sizeof and [PATCH] drivers/infiniband - standardize sizeof(foo) Joe Perches
2008-07-01 17:57 ` Roland Dreier
2008-07-01 17:37   ` Joe Perches
2008-07-01 19:21   ` [ofa-general] " Dotan Barak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox