GNU bug report logs - #15634
coreutils-8.21 possible coding error ?

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 15634 in the body.
You can then email your comments to 15634 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: David Binderman <dcb314 <at> hotmail.com>
To: "bug-coreutils <at> gnu.org" <bug-coreutils <at> gnu.org>
Subject: coreutils-8.21 possible coding error ?
Date: Thu, 17 Oct 2013 07:33:33 +0000

Hello there,

I just ran the static analysis tool cppcheck over the source code
of coreutils-8.21

It said many things, including

[src/dircolors.c:445]: (warning) Comparison of a boolean value using relational operator (<,>, <= or>=).

Source code is

  if (!print_database < argc)

Some round brackets might help clarify the code

  if ((!print_database) < argc)

or was perhaps

  if (!(print_database < argc))

intended ? Here are some other things cppcheck found.

[lib/mountlist.c:945]: (error) Resource leak: dirp
[lib/sig2str.c:329]: (warning) Logical conjunction always evaluates to false: signum <= -1 && signum>= 0.
[lib/closein.c:91]: (error) fflush() called on input stream 'stdin' results in undefined behaviour.

Regards

David Binderman 		 	   		  

Message #8 received at 15634 <at> debbugs.gnu.org (full text, mbox):

From: Eric Blake <eblake <at> redhat.com>
To: David Binderman <dcb314 <at> hotmail.com>, 15634 <at> debbugs.gnu.org
Subject: Re: bug#15634: coreutils-8.21 possible coding error ?
Date: Thu, 17 Oct 2013 05:19:44 -0600

[Message part 1 (text/plain, inline)]

On 10/17/2013 01:33 AM, David Binderman wrote:
> Hello there,
> 
> I just ran the static analysis tool cppcheck over the source code
> of coreutils-8.21

Thanks for reporting that.

> 
> It said many things, including
> 
> [src/dircolors.c:445]: (warning) Comparison of a boolean value using relational operator (<,>, <= or>=).
> 
> Source code is
> 
>   if (!print_database < argc)
> 
> Some round brackets might help clarify the code
> 
>   if ((!print_database) < argc)

This is the intended form; gcc doesn't warn about the precedence, but
silencing cppcheck may be worth it.

> 
> or was perhaps
> 
>   if (!(print_database < argc))
> 
> intended ? Here are some other things cppcheck found.
> 
> [lib/mountlist.c:945]: (error) Resource leak: dirp

This one has already been fixed in gnulib (commit 98171ec); it will be
fixed in coreutils the next time coreutils updates to latest gnulib.

> [lib/sig2str.c:329]: (warning) Logical conjunction always evaluates to false: signum <= -1 && signum>= 0.

Line 329 is:
    if (! (rtmin <= signum && signum <= rtmax))
with rtmin set to SIGRTMIN.  I'm not sure I follow how the error message
is claiming we are comparing signum <= -1.  Bug in cppcheck?

> [lib/closein.c:91]: (error) fflush() called on input stream 'stdin' results in undefined behaviour.

Bug in cppcheck.  Calling fflush(stdin) IS defined behavior, if stdin is
seekable (which the code prior to that line guaranteed).  POSIX 2013
explicitly says so:
http://pubs.opengroup.org/onlinepubs/9699919799/functions/fflush.html

[CX] [Option Start] For a stream open for reading, if the file is not
already at EOF, and the file is one capable of seeking, the file offset
of the underlying open file description shall be set to the file
position of the stream, and any characters pushed back onto the stream
by ungetc() or ungetwc() that have not subsequently been read from the
stream shall be discarded (without further changing the file offset).
[Option End]

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

[signature.asc (application/pgp-signature, attachment)]

Message #11 received at 15634 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: David Binderman <dcb314 <at> hotmail.com>, 15634 <at> debbugs.gnu.org
Subject: Re: bug#15634: coreutils-8.21 possible coding error ?
Date: Thu, 17 Oct 2013 07:09:48 -0700

David Binderman wrote:

> [lib/sig2str.c:329]: (warning) Logical conjunction always evaluates to false: signum <= -1 && signum>= 0.

This seems to be cppcheck complaining about a test that is needed
on other platforms, but which the compiler can optimize away on
your platform.  We can safely ignore this diagnostic as well:
it's perfectly OK, and in fact nice, to have code that a compiler
can optimize away on some platforms.

>   if (!print_database < argc)
> 
> Some round brackets might help clarify the code
> 
>   if ((!print_database) < argc)

The code's pretty clear as-is, since there
are spaces around the " < " but not after the "!".

I think I'd rather ignore this diagnostic; in general,
"Comparison of a boolean value using relational operator (<,>, <= or>=)"
is bogus.  There's nothing wrong with comparing Booleans.
If you have two Booleans A and B, and want to write "A implies B",
it's often faster and (once you get used to it) more comprehensible
to write "A <= B", instead of the "!A | B" that cppcheck would
seem to require.  (Admittedly I'd rather write "A -> B"
but this is C we're talking about....)

Eric already covered the other two diagnostics you mentioned.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.