bug#11467: Parfait problems with GNU coreutils

[Jim Meyering]

Rich Burridge wrote:
...
> I've attached a patch that we are applying to the code to fix these problems.
>
> Here's the evaluation of why the changes have been made:
>
> There are three different types of Parfait errors here:
>
> 1/ In fmt.c, in the get_paragraph() routine, Parfait thinks that there
>    is a potential problem of a negative index into the 'word' array.
>    But that situation is impossible. Earlier in the routine, get_line()
>    is called, and this has to have incremented word_limit. The solution
>    was to add a Parfait style comment before the offending line of code
>    to shut parfait up.
>
> 2/ In sort.c, there is an occurrence in the main() routine where 's' was
>    being dereferenced, but it could have been NULL. A check was added
>    to only do the dereferencing if that wasn't the case. It was noticed
>    that there was already similar code in the same routine, so this seems
>    a reasonable solution.
>
> 3/ In stty.c, there were two occurrences where bitsp was being dereferenced
>    that could have been a NULL pointer. In each case, a check was added
>    to only do the dereferencing if that wasn't the case. It was noticed
>    that there was already similar code in the sane_mode() routine, so this
>    seems a reasonable solution.

Thanks again.
I've just confirmed that your proposed stty.c change
is not required, since bitsp cannot be NULL when it is
dereferenced.

Are the following proposed changes enough to placate parfait?
I prefer to use assert, because that tends to work also for
static analysis tools like clang and coverity.

>From 94f417db5e093093ff9512869880e39975822be8 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Mon, 14 May 2012 15:44:41 +0200
Subject: [PATCH] maint: add assertions to placate static analysis tools

A static analysis tool (http://labs.oracle.com/projects/parfait/)
produced some false positive diagnostics.  Add assertions to help
it understand that the code is correct.
* src/stty.c: Include <assert.h>.
(display_changed): Add an assertion to placate parfait.
(display_all): Likewise.
* src/sort.c: Include <assert.h>.
(main): Add an assertion to placate parfait.
---
 src/sort.c | 5 +++++
 src/stty.c | 8 ++++++++
 2 files changed, 13 insertions(+)

diff --git a/src/sort.c b/src/sort.c
index 493e7f1..2593a2a 100644
--- a/src/sort.c
+++ b/src/sort.c
@@ -28,6 +28,7 @@
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <signal.h>
+#include <assert.h>
 #include "system.h"
 #include "argmatch.h"
 #include "error.h"
@@ -4243,6 +4244,10 @@ main (int argc, char **argv)
                           char const *optarg1 = argv[optind++];
                           s = parse_field_count (optarg1 + 1, &key->eword,
                                              N_("invalid number after '-'"));
+                          /* When called with a non-NULL message ID,
+                             parse_field_count cannot return NULL.  Tell static
+                             analysis tools that dereferencing S is safe.  */
+                          assert (s);
                           if (*s == '.')
                             s = parse_field_count (s + 1, &key->echar,
                                                N_("invalid number after '.'"));
diff --git a/src/stty.c b/src/stty.c
index eb07f85..a3fc3dd 100644
--- a/src/stty.c
+++ b/src/stty.c
@@ -52,6 +52,7 @@
 #endif
 #include <getopt.h>
 #include <stdarg.h>
+#include <assert.h>

 #include "system.h"
 #include "error.h"
@@ -1538,6 +1539,12 @@ display_changed (struct termios *mode)

       bitsp = mode_type_flag (mode_info[i].type, mode);
       mask = mode_info[i].mask ? mode_info[i].mask : mode_info[i].bits;
+
+      /* bitsp would be NULL only for "combination" modes, yet those
+         are filtered out above via the OMIT flag.  Tell static analysis
+         tools that it's ok to dereference bitsp here.  */
+      assert (bitsp);
+
       if ((*bitsp & mask) == mode_info[i].bits)
         {
           if (mode_info[i].flags & SANE_UNSET)
@@ -1615,6 +1622,7 @@ display_all (struct termios *mode, char const 
*device_name)

       bitsp = mode_type_flag (mode_info[i].type, mode);
       mask = mode_info[i].mask ? mode_info[i].mask : mode_info[i].bits;
+      assert (bitsp); /* See the identical assertion and comment above.  */
       if ((*bitsp & mask) == mode_info[i].bits)
         wrapf ("%s", mode_info[i].name);
       else if (mode_info[i].flags & REV)
--
1.7.10.2.484.gcd07cc5