Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1224870 > unrolled thread

[PATCH] coccinelle: tests: unsigned value cannot be lesser than zero

Back to article view | Back to linux.kernel

Contents

  [PATCH] coccinelle: tests: unsigned value cannot be lesser than zero Andrzej Hajda <a.hajda@samsung.com> - 2015-09-15 11:30 +0200
    Re: [PATCH] coccinelle: tests: unsigned value cannot be lesser than  zero Julia Lawall <julia.lawall@lip6.fr> - 2015-09-15 15:10 +0200
      Re: [PATCH] coccinelle: tests: unsigned value cannot be lesser than  zero SF Markus Elfring <elfring@users.sourceforge.net> - 2015-09-15 15:20 +0200
        Re: [PATCH] coccinelle: tests: unsigned value cannot be lesser than  zero Julia Lawall <julia.lawall@lip6.fr> - 2015-09-15 15:40 +0200
          Re: [PATCH] coccinelle: tests: unsigned value cannot be lesser than  zero Andrzej Hajda <a.hajda@samsung.com> - 2015-09-15 16:00 +0200
            Re: [PATCH] coccinelle: tests: unsigned value cannot be lesser than  zero Julia Lawall <julia.lawall@lip6.fr> - 2015-09-15 16:00 +0200
              Re: [PATCH] coccinelle: tests: unsigned value cannot be lesser than  zero Andrzej Hajda <a.hajda@samsung.com> - 2015-09-16 11:20 +0200
                Re: [PATCH] coccinelle: tests: unsigned value cannot be lesser than  zero Julia Lawall <julia.lawall@lip6.fr> - 2015-09-16 11:30 +0200
                  [PATCH v2] coccinelle: tests: unsigned value cannot be lesser than zero Andrzej Hajda <a.hajda@samsung.com> - 2015-09-16 15:30 +0200
                    Re: [PATCH v2] coccinelle: tests: unsigned value cannot be lesser  than zero Julia Lawall <julia.lawall@lip6.fr> - 2015-09-16 15:40 +0200
                    Re: [PATCH v2] coccinelle: tests: unsigned value cannot be lesser  than zero SF Markus Elfring <elfring@users.sourceforge.net> - 2015-09-16 21:00 +0200
    Re: [PATCH] coccinelle: tests: unsigned value cannot be lesser than  zero SF Markus Elfring <elfring@users.sourceforge.net> - 2015-09-15 15:10 +0200
      Re: [PATCH] coccinelle: tests: unsigned value cannot be lesser than  zero Andrzej Hajda <a.hajda@samsung.com> - 2015-09-15 15:50 +0200
        Re: [PATCH] coccinelle: tests: unsigned value cannot be lesser than  zero SF Markus Elfring <elfring@users.sourceforge.net> - 2015-09-15 16:40 +0200
          Re: [Cocci] [PATCH] coccinelle: tests: unsigned value cannot be  lesser than zero Julia Lawall <julia.lawall@lip6.fr> - 2015-09-15 16:50 +0200
            Re: [Cocci] [PATCH] coccinelle: tests: unsigned value cannot be  lesser than zero SF Markus Elfring <elfring@users.sourceforge.net> - 2015-09-15 17:00 +0200
        Re: [PATCH] coccinelle: tests: unsigned value cannot be lesser than  zero Julia Lawall <julia.lawall@lip6.fr> - 2015-09-18 07:40 +0200
          [PATCH v3] coccinelle: tests: unsigned value cannot be lesser than zero Andrzej Hajda <a.hajda@samsung.com> - 2015-09-21 12:40 +0200
            Re: [PATCH v3] coccinelle: tests: unsigned value cannot be lesser  than zero SF Markus Elfring <elfring@users.sourceforge.net> - 2015-09-21 15:10 +0200
              Re: [PATCH v3] coccinelle: tests: unsigned value cannot be lesser than  zero Andrzej Hajda <a.hajda@samsung.com> - 2015-09-21 15:40 +0200
                Re: [PATCH v3] coccinelle: tests: unsigned value cannot be lesser  than zero SF Markus Elfring <elfring@users.sourceforge.net> - 2015-09-21 16:10 +0200

Page 1 of 2  [1] 2  Next page →

#1224870 — [PATCH] coccinelle: tests: unsigned value cannot be lesser than zero

Code comparing unsigned variables with zero using operators < or >= does not
make sense. It is always false or true, respectively. However, its presence
often indicates bugs in the code.
gcc can detect it also using -Wtype-limits switch, but it warns also in correct
cases, making too much noise.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
Hi Julia,

This test finds 93 issues in kernel code (with --all-includes) which could
be corrected. Some of them are harmless, just unnecessary code, but there
are also serious bugs, like:
	u32 irq = platform_get_irq(...)
	if (irq < 0)
		...
	unsigned int target = cpumask_any_but(cpu_online_mask, cpu);
	if (target < 0)
		...

Regards
Andrzej
---
 .../tests/unsigned_lesser_than_zero.cocci          | 37 ++++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci

diff --git a/scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci b/scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci
new file mode 100644
index 0000000..6a90510
--- /dev/null
+++ b/scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci
@@ -0,0 +1,37 @@
+/// Unsigned variables cannot be lesser than zero. Presence of such checks
+/// can indicate incorrect variable type or just unnecessary code.
+///
+// Confidence: High
+// Copyright: (C) 2015 Andrzej Hajda, Samsung Electronics Co., Ltd. GPLv2.
+// URL: http://coccinelle.lip6.fr/
+// Options: --include-headers
+
+virtual context
+virtual org
+virtual report
+
+@r depends on context || org || report@
+position p;
+typedef u8, u16, u32, u64;
+{unsigned char, unsigned short int, unsigned int, unsigned long, unsigned long long, size_t, u8, u16, u32, u64} v;
+@@
+
+(
+*v@p < 0
+|
+*v@p >= 0
+)
+
+@script:python depends on org@
+p << r.p;
+@@
+
+msg = "WARNING: Unsigned value cannot be lesser than zero"
+coccilib.org.print_todo(p[0], msg)
+
+@script:python depends on report@
+p << r.p;
+@@
+
+msg = "WARNING: Unsigned value cannot be lesser than zero"
+coccilib.report.print_report(p[0], msg)
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1225076 — Re: [PATCH] coccinelle: tests: unsigned value cannot be lesser than zero

On Tue, 15 Sep 2015, SF Markus Elfring wrote:

> > +@r depends on context || org || report@
> > +position p;
> > +typedef u8, u16, u32, u64;
>
> Can the involved data types be restricted for unsigned types for such
> a source code analysis in a more general way?
>
>
> > +{unsigned char, unsigned short int, unsigned int, unsigned long, unsigned long long, size_t, u8, u16, u32, u64} v;
> > +@@
> > +
> > +(
> > +*v@p < 0
> > +|
> > +*v@p >= 0
> > +)
>
> How do you think about to use the following SmPL wording instead?
>
>  v@p
> (
> *< 0
> |
> *<= 0
> )

It does not, and is not intended to, work.  The branches of a disjunction
should be complete expressions.

julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1225091 — Re: [PATCH] coccinelle: tests: unsigned value cannot be lesser than zero

>>  v@p
>> (
>> *< 0
>> |
>> *<= 0
>> )
> 
> It does not, and is not intended to, work.  The branches of a disjunction
> should be complete expressions.

Will the following SmPL approach be more appropriate then?

(
*v@p < 0
|
*v@p <= 0
)

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1225098 — Re: [PATCH] coccinelle: tests: unsigned value cannot be lesser than zero

On Tue, 15 Sep 2015, SF Markus Elfring wrote:

> >>  v@p
> >> (
> >> *< 0
> >> |
> >> *<= 0
> >> )
> >
> > It does not, and is not intended to, work.  The branches of a disjunction
> > should be complete expressions.
>
> Will the following SmPL approach be more appropriate then?
>
> (
> *v@p < 0
> |
> *v@p <= 0
> )

Actually, all of

v < 0 (never true)
v <= 0 (same as v == 0)
v >= 0 (always true)

would seem to merit attention.  Andrzej, what do you think?

julia


>
> Regards,
> Markus
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1225127 — Re: [PATCH] coccinelle: tests: unsigned value cannot be lesser than zero

On 09/15/2015 03:31 PM, Julia Lawall wrote:
> On Tue, 15 Sep 2015, SF Markus Elfring wrote:
>
>>>>  v@p
>>>> (
>>>> *< 0
>>>> |
>>>> *<= 0
>>>> )
>>> It does not, and is not intended to, work.  The branches of a disjunction
>>> should be complete expressions.
>> Will the following SmPL approach be more appropriate then?
>>
>> (
>> *v@p < 0
>> |
>> *v@p <= 0
>> )
> Actually, all of
>
> v < 0 (never true)
> v <= 0 (same as v == 0)
> v >= 0 (always true)
>
> would seem to merit attention.  Andrzej, what do you think?

You are right, the 2nd case should be also addressed,
such code is misleading.
I will prepare then 2nd version of the patch.

Regards
Andrzej

>
> julia
>
>
>> Regards,
>> Markus
>> --
>> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1225134 — Re: [PATCH] coccinelle: tests: unsigned value cannot be lesser than zero

On Tue, 15 Sep 2015, Andrzej Hajda wrote:

> On 09/15/2015 03:31 PM, Julia Lawall wrote:
> > On Tue, 15 Sep 2015, SF Markus Elfring wrote:
> >
> >>>>  v@p
> >>>> (
> >>>> *< 0
> >>>> |
> >>>> *<= 0
> >>>> )
> >>> It does not, and is not intended to, work.  The branches of a disjunction
> >>> should be complete expressions.
> >> Will the following SmPL approach be more appropriate then?
> >>
> >> (
> >> *v@p < 0
> >> |
> >> *v@p <= 0
> >> )
> > Actually, all of
> >
> > v < 0 (never true)
> > v <= 0 (same as v == 0)
> > v >= 0 (always true)
> >
> > would seem to merit attention.  Andrzej, what do you think?
>
> You are right, the 2nd case should be also addressed,
> such code is misleading.
> I will prepare then 2nd version of the patch.

It could be reasonable to change the options to --all-includes?  Although
it could be somewhat slow.

julia

>
> Regards
> Andrzej
>
> >
> > julia
> >
> >
> >> Regards,
> >> Markus
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1225873 — Re: [PATCH] coccinelle: tests: unsigned value cannot be lesser than zero

On 09/15/2015 03:57 PM, Julia Lawall wrote:
> 
> 
> On Tue, 15 Sep 2015, Andrzej Hajda wrote:
> 
>> On 09/15/2015 03:31 PM, Julia Lawall wrote:
>>> On Tue, 15 Sep 2015, SF Markus Elfring wrote:
>>>
>>>>>>  v@p
>>>>>> (
>>>>>> *< 0
>>>>>> |
>>>>>> *<= 0
>>>>>> )
>>>>> It does not, and is not intended to, work.  The branches of a disjunction
>>>>> should be complete expressions.
>>>> Will the following SmPL approach be more appropriate then?
>>>>
>>>> (
>>>> *v@p < 0
>>>> |
>>>> *v@p <= 0
>>>> )
>>> Actually, all of
>>>
>>> v < 0 (never true)
>>> v <= 0 (same as v == 0)
>>> v >= 0 (always true)
>>>
>>> would seem to merit attention.  Andrzej, what do you think?
>>
>> You are right, the 2nd case should be also addressed,
>> such code is misleading.
>> I will prepare then 2nd version of the patch.
> 
> It could be reasonable to change the options to --all-includes?  Although
> it could be somewhat slow.

I have tested the patch with 'v <= 0', it spotted hundreds places with this
check. It seems to be quite common practice to use such checks with counters,
iterators, quantities, range checking. In fact it is negation of 'v > 0' which
seems to be acceptable even if it really means 'v != 0'. So maybe we should not
warn about it? What do you think?

On the other side it spotted also real bugs, but maybe I can make separate, more
specific test for such cases.

Regards
Andrzej

> 
> julia
> 
>>
>> Regards
>> Andrzej
>>
>>>
>>> julia
>>>
>>>
>>>> Regards,
>>>> Markus
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1225882 — Re: [PATCH] coccinelle: tests: unsigned value cannot be lesser than zero

On Wed, 16 Sep 2015, Andrzej Hajda wrote:

> On 09/15/2015 03:57 PM, Julia Lawall wrote:
> >
> >
> > On Tue, 15 Sep 2015, Andrzej Hajda wrote:
> >
> >> On 09/15/2015 03:31 PM, Julia Lawall wrote:
> >>> On Tue, 15 Sep 2015, SF Markus Elfring wrote:
> >>>
> >>>>>>  v@p
> >>>>>> (
> >>>>>> *< 0
> >>>>>> |
> >>>>>> *<= 0
> >>>>>> )
> >>>>> It does not, and is not intended to, work.  The branches of a disjunction
> >>>>> should be complete expressions.
> >>>> Will the following SmPL approach be more appropriate then?
> >>>>
> >>>> (
> >>>> *v@p < 0
> >>>> |
> >>>> *v@p <= 0
> >>>> )
> >>> Actually, all of
> >>>
> >>> v < 0 (never true)
> >>> v <= 0 (same as v == 0)
> >>> v >= 0 (always true)
> >>>
> >>> would seem to merit attention.  Andrzej, what do you think?
> >>
> >> You are right, the 2nd case should be also addressed,
> >> such code is misleading.
> >> I will prepare then 2nd version of the patch.
> >
> > It could be reasonable to change the options to --all-includes?  Although
> > it could be somewhat slow.
>
> I have tested the patch with 'v <= 0', it spotted hundreds places with this
> check. It seems to be quite common practice to use such checks with counters,
> iterators, quantities, range checking. In fact it is negation of 'v > 0' which
> seems to be acceptable even if it really means 'v != 0'. So maybe we should not
> warn about it? What do you think?

It seems a bit sloppy, but since the test does have some meaning, maybe it
is OK.

> On the other side it spotted also real bugs, but maybe I can make separate, more
> specific test for such cases.

OK, thanks.

julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1226076 — [PATCH v2] coccinelle: tests: unsigned value cannot be lesser than zero

Code comparing unsigned variables with zero using operators < or >= does not
make sense. It is always false or true, respectively. However, its presence
often indicates bugs in the code.
gcc can detect it also using -Wtype-limits switch, but it warns also in correct
cases, making too much noise.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
v2: added --all-includes option

As we discussed earlier I have dropped idea of adding v <= 0 as it is widely
used in checking ranges, counters, quantities.

Regards
Andrzej
---
 .../tests/unsigned_lesser_than_zero.cocci          | 37 ++++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci

diff --git a/scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci b/scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci
new file mode 100644
index 0000000..eab6d8c
--- /dev/null
+++ b/scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci
@@ -0,0 +1,37 @@
+/// Unsigned variables cannot be lesser than zero. Presence of such checks
+/// can indicate incorrect variable type or just unnecessary code.
+///
+// Confidence: High
+// Copyright: (C) 2015 Andrzej Hajda, Samsung Electronics Co., Ltd. GPLv2.
+// URL: http://coccinelle.lip6.fr/
+// Options: --include-headers --all-includes
+
+virtual context
+virtual org
+virtual report
+
+@r depends on context || org || report@
+position p;
+typedef u8, u16, u32, u64;
+{unsigned char, unsigned short int, unsigned int, unsigned long, unsigned long long, size_t, u8, u16, u32, u64} v;
+@@
+
+(
+*v@p < 0
+|
+*v@p >= 0
+)
+
+@script:python depends on org@
+p << r.p;
+@@
+
+msg = "WARNING: Unsigned value cannot be lesser than zero"
+coccilib.org.print_todo(p[0], msg)
+
+@script:python depends on report@
+p << r.p;
+@@
+
+msg = "WARNING: Unsigned value cannot be lesser than zero"
+coccilib.report.print_report(p[0], msg)
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1226092 — Re: [PATCH v2] coccinelle: tests: unsigned value cannot be lesser than zero

Acked-by: Julia Lawall <julia.lawall@lip6.fr>

On Wed, 16 Sep 2015, Andrzej Hajda wrote:

> Code comparing unsigned variables with zero using operators < or >= does not
> make sense. It is always false or true, respectively. However, its presence
> often indicates bugs in the code.
> gcc can detect it also using -Wtype-limits switch, but it warns also in correct
> cases, making too much noise.
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> v2: added --all-includes option
>
> As we discussed earlier I have dropped idea of adding v <= 0 as it is widely
> used in checking ranges, counters, quantities.
>
> Regards
> Andrzej
> ---
>  .../tests/unsigned_lesser_than_zero.cocci          | 37 ++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>  create mode 100644 scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci
>
> diff --git a/scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci b/scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci
> new file mode 100644
> index 0000000..eab6d8c
> --- /dev/null
> +++ b/scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci
> @@ -0,0 +1,37 @@
> +/// Unsigned variables cannot be lesser than zero. Presence of such checks
> +/// can indicate incorrect variable type or just unnecessary code.
> +///
> +// Confidence: High
> +// Copyright: (C) 2015 Andrzej Hajda, Samsung Electronics Co., Ltd. GPLv2.
> +// URL: http://coccinelle.lip6.fr/
> +// Options: --include-headers --all-includes
> +
> +virtual context
> +virtual org
> +virtual report
> +
> +@r depends on context || org || report@
> +position p;
> +typedef u8, u16, u32, u64;
> +{unsigned char, unsigned short int, unsigned int, unsigned long, unsigned long long, size_t, u8, u16, u32, u64} v;
> +@@
> +
> +(
> +*v@p < 0
> +|
> +*v@p >= 0
> +)
> +
> +@script:python depends on org@
> +p << r.p;
> +@@
> +
> +msg = "WARNING: Unsigned value cannot be lesser than zero"
> +coccilib.org.print_todo(p[0], msg)
> +
> +@script:python depends on report@
> +p << r.p;
> +@@
> +
> +msg = "WARNING: Unsigned value cannot be lesser than zero"
> +coccilib.report.print_report(p[0], msg)
> --
> 1.9.1
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1226393 — Re: [PATCH v2] coccinelle: tests: unsigned value cannot be lesser than zero

> As we discussed earlier I have dropped idea of adding v <= 0 as it is widely
> used in checking ranges, counters, quantities.

I find that such a design decision will need more fine-tuning of the suggested
small SmPL script.


> +@r depends on context || org || report@
> +position p;
> +typedef u8, u16, u32, u64;
> +{unsigned char, unsigned short int, unsigned int, unsigned long, unsigned long long, size_t, u8, u16, u32, u64} v;

Is it eventually needed to mention the key word "int" also together with the "long" data types?


> +@@
> +
> +(
> +*v@p < 0
> +|
> +*v@p >= 0
> +)

How do you think about to split this SmPL rule so that corresponding warning
messages will really fit?

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1225080 — Re: [PATCH] coccinelle: tests: unsigned value cannot be lesser than zero

> +@r depends on context || org || report@
> +position p;
> +typedef u8, u16, u32, u64;

Can the involved data types be restricted for unsigned types for such
a source code analysis in a more general way?


> +{unsigned char, unsigned short int, unsigned int, unsigned long, unsigned long long, size_t, u8, u16, u32, u64} v;
> +@@
> +
> +(
> +*v@p < 0
> +|
> +*v@p >= 0
> +)

How do you think about to use the following SmPL wording instead?

 v@p
(
*< 0
|
*<= 0
)

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1225122 — Re: [PATCH] coccinelle: tests: unsigned value cannot be lesser than zero

On 09/15/2015 03:01 PM, SF Markus Elfring wrote:
>> +@r depends on context || org || report@
>> +position p;
>> +typedef u8, u16, u32, u64;
> Can the involved data types be restricted for unsigned types for such
> a source code analysis in a more general way?

I am not sure if I understand correctly. If you think about removing all u*
typedefs it
will result in omitting u* related comparisons, unless you use
--recursive-includes option.
Another solution is to add '--include include/asm-generic/int-ll64.h' to kbuild,
surprisingly
this header file is common for all architectures :)

Regards
Andrzej

>
>
>> +{unsigned char, unsigned short int, unsigned int, unsigned long, unsigned long long, size_t, u8, u16, u32, u64} v;
>> +@@
>> +
>> +(
>> +*v@p < 0
>> +|
>> +*v@p >= 0
>> +)
> How do you think about to use the following SmPL wording instead?
>
>  v@p
> (
> *< 0
> |
> *<= 0
> )
>
> Regards,
> Markus
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1225210 — Re: [PATCH] coccinelle: tests: unsigned value cannot be lesser than zero

> If you think about removing all u* typedefs

I became interested in the use case to consider more type definitions
besides the ones which should usually be handled for Linux source files.


> it will result in omitting u* related comparisons,
> unless you use --recursive-includes option.

How do you think about to make this source code analysis parameter configurable?


>>> +{unsigned char, unsigned short int, unsigned int, unsigned long, unsigned long long, size_t, u8, u16, u32, u64} v;

How does the data type "size_t" fit into the suggested SmPL approach?

Would you like to reuse your approach for checking of more software eventually?

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1225261 — Re: [Cocci] [PATCH] coccinelle: tests: unsigned value cannot be lesser than zero

On Tue, 15 Sep 2015, SF Markus Elfring wrote:

> > If you think about removing all u* typedefs
>
> I became interested in the use case to consider more type definitions
> besides the ones which should usually be handled for Linux source files.
>
>
> > it will result in omitting u* related comparisons,
> > unless you use --recursive-includes option.
>
> How do you think about to make this source code analysis parameter configurable?

What parameter are you referring to?  --recursive-includes is already a
parameter.

> >>> +{unsigned char, unsigned short int, unsigned int, unsigned long, unsigned long long, size_t, u8, u16, u32, u64} v;
>
> How does the data type "size_t" fit into the suggested SmPL approach?

size_t is also unsigned.

> Would you like to reuse your approach for checking of more software
> eventually?

He is proposing a semantic patch for inclusion in the Linux kernel source
code, so it is not really necessary to consider types other than those
used by the Linux kernel.  People can modify the semantic patch if they
want for other uses.

julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1225271 — Re: [Cocci] [PATCH] coccinelle: tests: unsigned value cannot be lesser than zero

> --recursive-includes is already a parameter.

I am unsure if its effect on source code analysis speed will matter here.


> size_t is also unsigned.

Will such a specification work also without an explicit SmPL typedef?

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1227567 — Re: [PATCH] coccinelle: tests: unsigned value cannot be lesser than zero

 >> +{unsigned char, unsigned short int, unsigned int, unsigned long, unsigned long long, size_t, u8, u16, u32, u64} v;

How about adding bool?  I don't currently find any problems with it, but
perhaps some could arise.

julia


> >> +@@
> >> +
> >> +(
> >> +*v@p < 0
> >> +|
> >> +*v@p >= 0
> >> +)
> > How do you think about to use the following SmPL wording instead?
> >
> >  v@p
> > (
> > *< 0
> > |
> > *<= 0
> > )
> >
> > Regards,
> > Markus
> >
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1229196 — [PATCH v3] coccinelle: tests: unsigned value cannot be lesser than zero

Code comparing unsigned variables with zero using operators < or >= does not
make sense. It is always false or true, respectively. However, its presence
often indicates bugs in the code.
gcc can detect it also using -Wtype-limits switch, but it warns also in correct
cases, making too much noise.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Acked-by: Julia Lawall <julia.lawall@lip6.fr>
---
v3: added bool type
v2: added --all-includes option
---
 .../tests/unsigned_lesser_than_zero.cocci          | 37 ++++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci

diff --git a/scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci b/scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci
new file mode 100644
index 0000000..70e71c8
--- /dev/null
+++ b/scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci
@@ -0,0 +1,37 @@
+/// Unsigned variables cannot be lesser than zero. Presence of such checks
+/// can indicate incorrect variable type or just unnecessary code.
+///
+// Confidence: High
+// Copyright: (C) 2015 Andrzej Hajda, Samsung Electronics Co., Ltd. GPLv2.
+// URL: http://coccinelle.lip6.fr/
+// Options: --include-headers --all-includes
+
+virtual context
+virtual org
+virtual report
+
+@r depends on context || org || report@
+position p;
+typedef bool, u8, u16, u32, u64;
+{unsigned char, unsigned short int, unsigned int, unsigned long, unsigned long long, size_t, bool, u8, u16, u32, u64} v;
+@@
+
+(
+*v@p < 0
+|
+*v@p >= 0
+)
+
+@script:python depends on org@
+p << r.p;
+@@
+
+msg = "WARNING: Unsigned value cannot be lesser than zero"
+coccilib.org.print_todo(p[0], msg)
+
+@script:python depends on report@
+p << r.p;
+@@
+
+msg = "WARNING: Unsigned value cannot be lesser than zero"
+coccilib.report.print_report(p[0], msg)
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1229275 — Re: [PATCH v3] coccinelle: tests: unsigned value cannot be lesser than zero

> v3: added bool type

I would appreciate a bit more feedback for my concerns around your
evolving approach.
* Reuse of "long int"?
* Splitting of the suggested SmPL rule so that each source code check
will be connected with appropriate warning messages.

Will any more fine-tuning be useful?

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1229318 — Re: [PATCH v3] coccinelle: tests: unsigned value cannot be lesser than zero

On 09/21/2015 03:02 PM, SF Markus Elfring wrote:
>> v3: added bool type
> I would appreciate a bit more feedback for my concerns around your
> evolving approach.

Ups, I have missed your email.

> * Reuse of "long int"?
If you mean adding int to 'unsigned long [long]' types, it does not work.
For some reason it works only without adding int after long.

> * Splitting of the suggested SmPL rule so that each source code check
> will be connected with appropriate warning messages.

Personally I prefer one message as it is more compact and
fits quite well in both cases, but I have no strong fillings with separate
message for each case.

>
> Will any more fine-tuning be useful?
Could you elaborate it.

Regards
Andrzej

>
> Regards,
> Markus
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web