bug-groff
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug #43569] Fix for compile warnings with gcc 4.6.3


From: Ingo Schwarze
Subject: [bug #43569] Fix for compile warnings with gcc 4.6.3
Date: Fri, 14 Nov 2014 20:45:41 +0000
User-agent: Mozilla/5.0 (X11; OpenBSD i386; rv:33.0) Gecko/20100101 Firefox/33.0

Follow-up Comment #8, bug #43569 (project groff):

> The code I found HAD a cast to (void):
> (void) fscanf(file, "%" MAXSTRING_S "s%*[^n]n", string);
> But nevertheless gcc issued the warning.
> That's why I submitted the patch. 

Merely looking at formal aspects of the code and shutting up the compiler is
worse than useless, you need to understand what it does and identify and fix
actual bugs.

For example, the specific fscanf() call you are citing *is* buggy.  If the
gremlinfile is completely empty - as in:

  .GS
  file /dev/null
  .GE

then fscanf returns EOF, which is ignored, and the uninitialized content of
string[] is inspected.  If that, by unlucky chance, happens to be
"gremlinfile", the wrong error message gets printed: "error in file format"
instead of "is not a gremlin file".

Arguably, that's not a very severe error.  The next one, caused by one of the
other unchecked fscanf()s you are sweeping under the carpet, is worse.  If the
first coordinate line starts with a non-numeric character, the problem is
ignored, and x and y are used uninitialized.  Try the following gremlinfile:

  gremlinfile
  1 0.0 0.0
  6
  x100.0 100.0
  100.0 200.0
  200.0 200.0
  200.0 100.0
  -1.0 -1.0
  5 1
  0
  -1

The upper left corner of the square ends up in a random location.

Besides, by putting invalid content into the gremlinfile, it's easy to provoke
segfaults; i didn't look deeper into that, but that would no doubt be
worthwhile.

THIS CODE IS BUGGY AS HELL.  By all means, fix it if you want to.  But don't
blindly put lipstick on the pig, merely hiding the bugs.

Don't blindly believe in static analysis tools (including compilers).  They
are not gods.  Sometimes, their rantings make little sense.  They are mostly
useful to detect subtle, well-hidden issues in code that already is of good
quality - but even then, you have to ignore a false positive now and then. 
When dealing with crap that was never audited at all, you find more errors by
merely reading the code, without letting any static analysis tool distract
you.

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?43569>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/




reply via email to

[Prev in Thread] Current Thread [Next in Thread]