Skip to contentSkip to conversation

- scripts/CodingStyle_sizeof - standardize sizeof and [PATCH] drivers/infiniband - standardize sizeof(foo)

RFC PATCH In review Joe Perches 3 participants 4 messages Jul 1, 2008

lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/1 - scripts/CodingStyle_sizeof - standardize sizeof and [PATCH] drivers/infiniband - standardize sizeof(foo)

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
---
 scripts/CodingStyle_sizeof |   33 +++++++++++++++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)
scripts/CodingStyle_sizeof new file +33 -0
new file mode 100755
index 0000000..7f2444f
@@ -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
#1 lore raw

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.

#2 lore raw
Quoted 4 lines · Roland Dreier

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

#3 lore raw
In reply to Roland Dreier
Quoted 5 lines · Roland Dreier

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