Fix some (harmless) cppcheck warnings.

[dwarf_getaranges.c:149]: (warning) Ineffective statement similar to '*A++;'.
Did you intend to write '(*A)++;'?

There was already an XXX statement that we weren't using the result.
Explicitly read the segment_size and check it is zero. And report an
error if it isn't, since we aren't prepared to handle such a case.

[arlib.c:62]: (error) Uninitialized variable: tmpbuf
[arlib.c:124]: (error) Uninitialized variable: tmpbuf

cppcheck is wrong. tmpbuf is initialized in the snprintf call whose result
is use in the same memcpy call. It does make the code less readable and
harder to understand. So explicitly split the snprintf and memcpy calls.

[nm.c:766]: (warning) Assert statement modifies 'cnt'.

The cnt variable was only used for this assert sanity check.
But it is bad style to do have side effects inside an assert statement.
Increase cnt after the assert.

Reported-by: David Binderman <dcb314@hotmail.com>
Signed-off-by: Mark Wielaard <mjw@redhat.com>

diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index c355c306c88120fb0521446b99eda56c2c4d4ab2..dc0c4c9004b9df51475948c7a117c2c48de95b40 100644 (file)
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,8 @@
+2013-11-09  Mark Wielaard  <mjw@redhat.com>
+
+       * dwarf_getaranges.c (dwarf_getaranges): Read segment_size and
+       check that it is zero.
+
 2013-11-07  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
        * cfi.h (struct Dwarf_Frame_s): Make the comment more specific.

diff --git a/libdw/dwarf_getaranges.c b/libdw/dwarf_getaranges.c
index cc70cb21108674a983e10c62cd792fa321f05659..20ac7ec6781cdd9e09f4481e666c5f7760383fd8 100644 (file)
--- a/libdw/dwarf_getaranges.c
+++ b/libdw/dwarf_getaranges.c
@@ -144,9 +144,10 @@ dwarf_getaranges (dbg, aranges, naranges)
       if (address_size != 4 && address_size != 8)
        goto invalid;
 
-      /* Ignore the segment size value.  */
-      // XXX Really?
-      (void) *readp++;
+      /* We don't actually support segment selectors.  */
+      unsigned int segment_size = *readp++;
+      if (segment_size != 0)
+       goto invalid;
 
       /* Round the address to the next multiple of 2*address_size.  */
       readp += ((2 * address_size - ((readp - hdrstart) % (2 * address_size)))

diff --git a/src/ChangeLog b/src/ChangeLog
index aebcb2ff68c70de96c78ced980facaf1e6b0e71f..94bc27a951f3c94c795f7ad6a040a69cc7edef88 100644 (file)
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,10 @@
+2013-11-09  Mark Wielaard  <mjw@redhat.com>
+
+       * arlib.c (arlib_init): Call snprintf before using the result
+       with memcpy.
+       (arlib_finalize): Likewise.
+       * nm.c (show_symbols_sysv): Don't modify cnt inside assert.
+
 2013-11-07  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
        * Makefile.am (bin_PROGRAMS): Add stack.

diff --git a/src/arlib.c b/src/arlib.c
index 62c517cca084e0ec0c34eb86fd0a3ec49b2a532e..43a9145b71a6a0a9c7ff691313e81bcc496d72f8 100644 (file)
--- a/src/arlib.c
+++ b/src/arlib.c
@@ -59,11 +59,11 @@ arlib_init (void)
      _FORTIFY_SOURCE=2 would not let us play these games.  Therefore
      we play it safe.  */
   char tmpbuf[sizeof (ar_hdr.ar_date) + 1];
-  memcpy (ar_hdr.ar_date, tmpbuf,
-         snprintf (tmpbuf, sizeof (tmpbuf), "%-*lld",
+  int s = snprintf (tmpbuf, sizeof (tmpbuf), "%-*lld",
                    (int) sizeof (ar_hdr.ar_date),
                     (arlib_deterministic_output ? 0
-                     : (long long int) time (NULL))));
+                     : (long long int) time (NULL)));
+  memcpy (ar_hdr.ar_date, tmpbuf, s);
   assert ((sizeof (struct ar_hdr)  % sizeof (uint32_t)) == 0);
 
   /* Note the string for the ar_uid and ar_gid cases is longer than
@@ -121,10 +121,10 @@ arlib_finalize (void)
 
       symtab.longnames = obstack_finish (&symtab.longnamesob);
 
-      memcpy (&((struct ar_hdr *) symtab.longnames)->ar_size, tmpbuf,
-             snprintf (tmpbuf, sizeof (tmpbuf), "%-*zu",
+      int s = snprintf (tmpbuf, sizeof (tmpbuf), "%-*zu",
                        (int) sizeof (((struct ar_hdr *) NULL)->ar_size),
-                       symtab.longnameslen - sizeof (struct ar_hdr)));
+                       symtab.longnameslen - sizeof (struct ar_hdr));
+      memcpy (&((struct ar_hdr *) symtab.longnames)->ar_size, tmpbuf, s);
     }
 
   symtab.symsofflen = obstack_object_size (&symtab.symsoffob);

diff --git a/src/nm.c b/src/nm.c
index 7aae84b0d1ce6193dc1e625747ed5dd16080b7f5..d434f44e81a7c4e17542fbff809616934eaf1fc7 100644 (file)
--- a/src/nm.c
+++ b/src/nm.c
@@ -763,7 +763,8 @@ show_symbols_sysv (Ebl *ebl, GElf_Word strndx, const char *fullname,
     {
       GElf_Shdr shdr_mem;
 
-      assert (elf_ndxscn (scn) == cnt++);
+      assert (elf_ndxscn (scn) == cnt);
+      cnt++;
 
       char *name = elf_strptr (ebl->elf, shstrndx,
                               gelf_getshdr (scn, &shdr_mem)->sh_name);