2013-10-07 23:45:31

by Joe Perches

[permalink] [raw]
Subject: [PATCH] checkpatch: Add check for sscanf without return use

Naked use sscanf can be troublesome.
Add a warning when the sscanf return value is not used.

Signed-off-by: Joe Perches <[email protected]>
---
scripts/checkpatch.pl | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 23d55bf..ba8af49 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -4012,6 +4012,23 @@ sub process {
}
}

+# check for naked sscanf
+ if ($^V && $^V ge 5.10.0 &&
+ defined $stat &&
+ $stat =~ /\bsscanf\b/ &&
+ ($stat !~ /$Ident\s*=\s*sscanf\s*$balanced_parens/ &&
+ $stat !~ /\bsscanf\s*$balanced_parens\s*(?:==|\!=)/ &&
+ $stat !~ /(?:==|\!=)\s*\bsscanf\s*$balanced_parens/)) {
+ my $lc = $stat =~ tr@\n@@;
+ $lc = $lc + $linenr;
+ my $stat_real = raw_line($linenr, 0);
+ for (my $count = $linenr + 1; $count <= $lc; $count++) {
+ $stat_real = $stat_real . "\n" . raw_line($count, 0);
+ }
+ WARN("NAKED_SSCANF",
+ "unchecked sscanf return value\n" . "$here\n$stat_real\n");
+ }
+
# check for new externs in .h files.
if ($realfile =~ /\.h$/ &&
$line =~ /^\+\s*(extern\s+)$Type\s*$Ident\s*\(/s) {


2013-10-07 23:56:29

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] checkpatch: Add check for sscanf without return use

On Mon, 07 Oct 2013 16:45:23 -0700 Joe Perches <[email protected]> wrote:

> Naked use sscanf can be troublesome.

It would be helpful to expand on "troublesome"?

> Add a warning when the sscanf return value is not used.

Maybe it should be __must_check?

2013-10-08 00:03:19

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] checkpatch: Add check for sscanf without return use

On Mon, 2013-10-07 at 16:56 -0700, Andrew Morton wrote:
> On Mon, 07 Oct 2013 16:45:23 -0700 Joe Perches <[email protected]> wrote:
>
> > Naked use sscanf can be troublesome.
>
> It would be helpful to expand on "troublesome"?

Dunno, you either know or the couple of paragraphs
it takes to explain with examples won't really be
too helpful in a commit log.

> > Add a warning when the sscanf return value is not used.
> Maybe it should be __must_check?

Maybe.

That might be easier but there'll be a couple
hundred new build warnings.

2013-10-08 00:11:16

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH] checkpatch: Add check for sscanf without return use

On Mon, Oct 7, 2013 at 6:03 PM, Joe Perches <[email protected]> wrote:
> On Mon, 2013-10-07 at 16:56 -0700, Andrew Morton wrote:
>> On Mon, 07 Oct 2013 16:45:23 -0700 Joe Perches <[email protected]> wrote:
>>
>> > Naked use sscanf can be troublesome.
>>
>> It would be helpful to expand on "troublesome"?
>
> Dunno, you either know or the couple of paragraphs
> it takes to explain with examples won't really be
> too helpful in a commit log.

It would be helpful to me, since I don't write code with sscanf every
day. A URL to an LKML description or something would be enough if the
commit log size is the concern.

Bjorn

2013-10-08 00:29:09

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] checkpatch: Add check for sscanf without return use

On Mon, 2013-10-07 at 18:10 -0600, Bjorn Helgaas wrote:
> On Mon, Oct 7, 2013 at 6:03 PM, Joe Perches <[email protected]> wrote:
> > On Mon, 2013-10-07 at 16:56 -0700, Andrew Morton wrote:
> >> On Mon, 07 Oct 2013 16:45:23 -0700 Joe Perches <[email protected]> wrote:
> >>
> >> > Naked use sscanf can be troublesome.
> >>
> >> It would be helpful to expand on "troublesome"?
> >
> > Dunno, you either know or the couple of paragraphs
> > it takes to explain with examples won't really be
> > too helpful in a commit log.
>
> It would be helpful to me, since I don't write code with sscanf every
> day. A URL to an LKML description or something would be enough if the
> commit log size is the concern.

I sent a V2 patch to Andrew and mm using this commit log:

Naked use sscanf can be troublesome because the pointed
to variables may not have been set.

Add a warning when the sscanf return value is not used.

Signed-off-by: Joe Perches <[email protected]>
---
V2: Use the $Compare variable instead of just == or !=
There are some tests that use < and >

2013-10-08 00:37:30

by Geyslan G. Bem

[permalink] [raw]
Subject: Re: [PATCH] checkpatch: Add check for sscanf without return use

2013/10/7 Joe Perches <[email protected]>:
> On Mon, 2013-10-07 at 18:10 -0600, Bjorn Helgaas wrote:
>> On Mon, Oct 7, 2013 at 6:03 PM, Joe Perches <[email protected]> wrote:
>> > On Mon, 2013-10-07 at 16:56 -0700, Andrew Morton wrote:
>> >> On Mon, 07 Oct 2013 16:45:23 -0700 Joe Perches <[email protected]> wrote:
>> >>

>
> Add a warning when the sscanf return value is not used.
>

It'll be nice.