Using the BIT() macro instead of manually shifting bits
makes the code less error prone and also more readable.
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
An example of the patches that can be obtained with this spatch:
http://www.mail-archive.com/[email protected]/msg02722.html
scripts/coccinelle/api/bit.cocci | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
create mode 100644 scripts/coccinelle/api/bit.cocci
diff --git a/scripts/coccinelle/api/bit.cocci b/scripts/coccinelle/api/bit.cocci
new file mode 100644
index 0000000..a5df73a
--- /dev/null
+++ b/scripts/coccinelle/api/bit.cocci
@@ -0,0 +1,25 @@
+// Use the macro BIT() macro if possible
+//
+// Confidence: High
+// Copyright (C) 2014 Javier Martinez Canillas. GPLv2.
+// URL: http://coccinelle.lip6.fr/
+// Options: --include-headers
+
+@hasbitops@
+@@
+
+#include <linux/bitops.h>
+
+@depends on hasbitops@
+expression E;
+@@
+
+- 1 << E
++ BIT(E)
+
+@depends on hasbitops@
+expression E;
+@@
+
+- BIT((E))
++ BIT(E)
--
1.9.1
On Sun, Apr 27, 2014 at 02:29:46AM +0200, Javier Martinez Canillas wrote:
> Using the BIT() macro instead of manually shifting bits
> makes the code less error prone and also more readable.
Does it? It is a taste thing, yet I don't think it makes the code that
much more readable that it is worth changing the whole tree.
Hello Wolfram,
Thanks a lot for your feedback.
On Sun, Apr 27, 2014 at 7:14 AM, Wolfram Sang <[email protected]> wrote:
> On Sun, Apr 27, 2014 at 02:29:46AM +0200, Javier Martinez Canillas wrote:
>> Using the BIT() macro instead of manually shifting bits
>> makes the code less error prone and also more readable.
>
> Does it? It is a taste thing, yet I don't think it makes the code that
> much more readable that it is worth changing the whole tree.
>
I believe there is a reason for that macro but yes I agree with you
that is a matter of taste and the it shouldn't be enforced.
I'm doing a big refactoring for the GPIO subsystem and was told to use
coccinelle so this patch was part of my learning. I posted it because
I thought that it could be useful but I don't mind the patch to be
dropped if that is not the case.
Best regards,
Javier
On Sun, 27 Apr 2014, Javier Martinez Canillas wrote:
> Hello Wolfram,
>
> Thanks a lot for your feedback.
>
> On Sun, Apr 27, 2014 at 7:14 AM, Wolfram Sang <[email protected]> wrote:
> > On Sun, Apr 27, 2014 at 02:29:46AM +0200, Javier Martinez Canillas wrote:
> >> Using the BIT() macro instead of manually shifting bits
> >> makes the code less error prone and also more readable.
> >
> > Does it? It is a taste thing, yet I don't think it makes the code that
> > much more readable that it is worth changing the whole tree.
> >
>
> I believe there is a reason for that macro but yes I agree with you
> that is a matter of taste and the it shouldn't be enforced.
>
> I'm doing a big refactoring for the GPIO subsystem and was told to use
> coccinelle so this patch was part of my learning. I posted it because
> I thought that it could be useful but I don't mind the patch to be
> dropped if that is not the case.
Perhaps it could be useful in files that already use BIT somewhere?
julia
Hello Julia,
On Sun, Apr 27, 2014 at 11:36 AM, Julia Lawall <[email protected]> wrote:
>
>
> On Sun, 27 Apr 2014, Javier Martinez Canillas wrote:
>
>> Hello Wolfram,
>>
>> Thanks a lot for your feedback.
>>
>> On Sun, Apr 27, 2014 at 7:14 AM, Wolfram Sang <[email protected]> wrote:
>> > On Sun, Apr 27, 2014 at 02:29:46AM +0200, Javier Martinez Canillas wrote:
>> >> Using the BIT() macro instead of manually shifting bits
>> >> makes the code less error prone and also more readable.
>> >
>> > Does it? It is a taste thing, yet I don't think it makes the code that
>> > much more readable that it is worth changing the whole tree.
>> >
>>
>> I believe there is a reason for that macro but yes I agree with you
>> that is a matter of taste and the it shouldn't be enforced.
>>
>> I'm doing a big refactoring for the GPIO subsystem and was told to use
>> coccinelle so this patch was part of my learning. I posted it because
>> I thought that it could be useful but I don't mind the patch to be
>> dropped if that is not the case.
>
> Perhaps it could be useful in files that already use BIT somewhere?
>
Well the semantic patch already has a rule that checks if the file
includes <linux/bitops.h> so files that don't include this header will
be skipped.
I've checked and in most cases when that header is included is because
at least the BIT macro is used once on the file. My guess is that the
original author included the header and used the macro but other
people modifying the file after its original creation just used 1 << n
instead.
But as I said, I've no strong opinion about this patch. I just used to
learn the basics of SmPL and to cleanup a driver I maintain and
thought it was a good touch to post it in case more people find it
useful.
> julia
Thanks a lot and best regards,
Javier
On Sun, 27 Apr 2014, Javier Martinez Canillas wrote:
> Using the BIT() macro instead of manually shifting bits
> makes the code less error prone and also more readable.
>
> Signed-off-by: Javier Martinez Canillas <[email protected]>
> ---
>
> An example of the patches that can be obtained with this spatch:
>
> http://www.mail-archive.com/[email protected]/msg02722.html
>
> scripts/coccinelle/api/bit.cocci | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
> create mode 100644 scripts/coccinelle/api/bit.cocci
>
> diff --git a/scripts/coccinelle/api/bit.cocci b/scripts/coccinelle/api/bit.cocci
> new file mode 100644
> index 0000000..a5df73a
> --- /dev/null
> +++ b/scripts/coccinelle/api/bit.cocci
> @@ -0,0 +1,25 @@
> +// Use the macro BIT() macro if possible
> +//
> +// Confidence: High
> +// Copyright (C) 2014 Javier Martinez Canillas. GPLv2.
> +// URL: http://coccinelle.lip6.fr/
> +// Options: --include-headers
> +
> +@hasbitops@
> +@@
> +
> +#include <linux/bitops.h>
Here you could say:
@usesbit@
@@
BIT(...)
> +@depends on hasbitops@
and then here it would be
@depends on hasbitops && usesbit@
julia
> +expression E;
> +@@
> +
> +- 1 << E
> ++ BIT(E)
> +
> +@depends on hasbitops@
> +expression E;
> +@@
> +
> +- BIT((E))
> ++ BIT(E)
> --
> 1.9.1
>
>
On Sun, Apr 27, 2014 at 12:29 PM, Julia Lawall <[email protected]> wrote:
>
>
> On Sun, 27 Apr 2014, Javier Martinez Canillas wrote:
>
>> Using the BIT() macro instead of manually shifting bits
>> makes the code less error prone and also more readable.
>>
>> Signed-off-by: Javier Martinez Canillas <[email protected]>
>> ---
>>
>> An example of the patches that can be obtained with this spatch:
>>
>> http://www.mail-archive.com/[email protected]/msg02722.html
>>
>> scripts/coccinelle/api/bit.cocci | 25 +++++++++++++++++++++++++
>> 1 file changed, 25 insertions(+)
>> create mode 100644 scripts/coccinelle/api/bit.cocci
>>
>> diff --git a/scripts/coccinelle/api/bit.cocci b/scripts/coccinelle/api/bit.cocci
>> new file mode 100644
>> index 0000000..a5df73a
>> --- /dev/null
>> +++ b/scripts/coccinelle/api/bit.cocci
>> @@ -0,0 +1,25 @@
>> +// Use the macro BIT() macro if possible
>> +//
>> +// Confidence: High
>> +// Copyright (C) 2014 Javier Martinez Canillas. GPLv2.
>> +// URL: http://coccinelle.lip6.fr/
>> +// Options: --include-headers
>> +
>> +@hasbitops@
>> +@@
>> +
>> +#include <linux/bitops.h>
>
> Here you could say:
>
> @usesbit@
> @@
> BIT(...)
>
>
>> +@depends on hasbitops@
>
> and then here it would be
>
> @depends on hasbitops && usesbit@
>
> julia
>
Thanks a lot for the feedback, I'll send a v2 of the patch then.
Best regards,
Javier