[Bug #39158] Source cleanups suggested by cppcheck utility.

diff --git a/ChangeLog b/ChangeLog
index 8454b08cf1919839b9225030897d003095bc2734..608f8702921bacc6addf3590fd2a2229f4e74b77 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,25 @@
 2013-07-21  Paul Smith  <psmith@gnu.org>
 
+       Cleanups detected by cppcheck.  Fixes Savannah bug #39158.
+       * arscan.c (ar_scan): Reduce the scope of local variables.
+       * dir.c (vms_hash): Ditto.
+       (find_directory): Ditto.
+       (file_impossible_p): Ditto.
+       * expand.c (variable_expand_string): Ditto.
+       * function.c (func_sort): Ditto.
+       (func_and): Ditto.
+       * job.c (reap_children): Ditto.
+       (exec_command): Ditto.
+       * main.c (main): Ditto.
+       * misc.c (collapse_continuations): Ditto.
+       * read.c (eval): Ditto.
+       (parse_file_seq): Ditto.
+       * vpath.c (gpath_search): Ditto.
+       (selective_vpath_search): Ditto.
+       * job.c (is_bourne_compatible_shell): Simplify for non-Windows systems.
+       * remake.c (f_mtime): Remove duplicate test.
+       * signame.c (strsignal): Fix bogus conditional.
+
        * job.c (assign_child_tempfiles): Assign OUTFD to -1 for safety.
        (start_job_command): Don't test output_sync and sync_cmd: redundant.
        Changes suggested by Frank Heckenbach <f.heckenbach@fh-soft.de>.

diff --git a/arscan.c b/arscan.c
index afdb6016de41b62908f1c2caa4710b477f064651..2b3cd5dfb00decec29f66af79e08fc8da9a03803 100644 (file)
--- a/arscan.c
+++ b/arscan.c
@@ -328,12 +328,10 @@ ar_scan (const char *archive, ar_member_func_t function, const void *arg)
 {
 #ifdef AIAMAG
   FL_HDR fl_header;
-#ifdef AIAMAGBIG
+# ifdef AIAMAGBIG
   int big_archive = 0;
   FL_HDR_BIG fl_header_big;
-#endif
-#else
-  int long_name = 0;
+# endif
 #endif
   char *namemap = 0;
   int desc = open (archive, O_RDONLY, 0);
@@ -461,6 +459,7 @@ ar_scan (const char *archive, ar_member_func_t function, const void *arg)
         char namebuf[sizeof member_header.ar_name + 1];
         char *name;
         int is_namemap;         /* Nonzero if this entry maps long names.  */
+        int long_name = 0;
 #endif
         long int eltsize;
         int eltmode;

diff --git a/dir.c b/dir.c
index 5681275e68cd936ddc573ca9cdbb754129e69677..0c93ac9c4d81b0f98d233ddd933726baef93ec7b 100644 (file)
--- a/dir.c
+++ b/dir.c
@@ -145,11 +145,11 @@ static int
 vms_hash (const char *name)
 {
   int h = 0;
-  int g;
 
   while (*name)
     {
       unsigned char uc = *name;
+      int g;
 #ifdef HAVE_CASE_INSENSITIVE_FS
       h = (h << 4) + (isupper (uc) ? tolower (uc) : uc);
 #else
@@ -414,19 +414,10 @@ static struct directory *find_directory (const char *name);
 static struct directory *
 find_directory (const char *name)
 {
-  const char *p;
   struct directory *dir;
   struct directory **dir_slot;
   struct directory dir_key;
-  int r;
-#ifdef WINDOWS32
-  char* w32_path;
-  char  fs_label[BUFSIZ];
-  char  fs_type[BUFSIZ];
-  unsigned long  fs_serno;
-  unsigned long  fs_flags;
-  unsigned long  fs_len;
-#endif
+
 #ifdef VMS
   if ((*name == '.') && (*(name+1) == 0))
     name = "[]";
@@ -440,11 +431,11 @@ find_directory (const char *name)
 
   if (HASH_VACANT (dir))
     {
-      struct stat st;
-
       /* The directory was not found.  Create a new entry for it.  */
+      const char *p = name + strlen (name);
+      struct stat st;
+      int r;
 
-      p = name + strlen (name);
       dir = xmalloc (sizeof (struct directory));
 #if defined(HAVE_CASE_INSENSITIVE_FS) && defined(VMS)
       dir->name = strcache_add_len (downcase (name), p - name);
@@ -488,6 +479,9 @@ find_directory (const char *name)
         {
           /* Search the contents hash table; device and inode are the key.  */
 
+#ifdef WINDOWS32
+          char *w32_path;
+#endif
           struct directory_contents *dc;
           struct directory_contents **dc_slot;
           struct directory_contents dc_key;
@@ -511,7 +505,13 @@ find_directory (const char *name)
           if (HASH_VACANT (dc))
             {
               /* Nope; this really is a directory we haven't seen before.  */
-
+#ifdef WINDOWS32
+              char  fs_label[BUFSIZ];
+              char  fs_type[BUFSIZ];
+              unsigned long  fs_serno;
+              unsigned long  fs_flags;
+              unsigned long  fs_len;
+#endif
               dc = (struct directory_contents *)
                 xmalloc (sizeof (struct directory_contents));
 
@@ -522,11 +522,8 @@ find_directory (const char *name)
               dc->ctime = st.st_ctime;
               dc->mtime = st.st_mtime;
 
-              /*
-               * NTFS is the only WINDOWS32 filesystem that bumps mtime
-               * on a directory when files are added/deleted from
-               * a directory.
-               */
+              /* NTFS is the only WINDOWS32 filesystem that bumps mtime on a
+                 directory when files are added/deleted from a directory.  */
               w32_path[3] = '\0';
               if (GetVolumeInformation (w32_path, fs_label, sizeof (fs_label),
                                         &fs_serno, &fs_len, &fs_flags, fs_type,
@@ -894,7 +891,6 @@ int
 file_impossible_p (const char *filename)
 {
   const char *dirend;
-  const char *p = filename;
   struct directory_contents *dir;
   struct dirfile *dirfile;
   struct dirfile dirfile_key;
@@ -939,12 +935,12 @@ file_impossible_p (const char *filename)
             dirend++;
 #endif
           cp = alloca (dirend - filename + 1);
-          memcpy (cp, p, dirend - p);
-          cp[dirend - p] = '\0';
+          memcpy (cp, filename, dirend - filename);
+          cp[dirend - filename] = '\0';
           dirname = cp;
         }
       dir = find_directory (dirname)->contents;
-      p = filename = slash + 1;
+      filename = slash + 1;
     }
 
   if (dir == 0 || dir->dirfiles.ht_vec == 0)
@@ -952,13 +948,13 @@ file_impossible_p (const char *filename)
     return 0;
 
 #ifdef __MSDOS__
-  filename = dosify (p);
+  filename = dosify (filename);
 #endif
 #ifdef HAVE_CASE_INSENSITIVE_FS
-  filename = downcase (p);
+  filename = downcase (filename);
 #endif
 #ifdef VMS
-  filename = vmsify (p, 1);
+  filename = vmsify (filename, 1);
 #endif
 
   dirfile_key.name = filename;

diff --git a/expand.c b/expand.c
index 4eabcc64c62ca45ca35a7fc39929f579b3a16fb3..ba04e48430df10af9733184b1ca116ec3316c119 100644 (file)
--- a/expand.c
+++ b/expand.c
@@ -304,10 +304,8 @@ variable_expand_string (char *line, const char *string, long length)
             if (colon)
               {
                 /* This looks like a substitution reference: $(FOO:A=B).  */
-                const char *subst_beg, *subst_end, *replace_beg, *replace_end;
-
-                subst_beg = colon + 1;
-                subst_end = lindex (subst_beg, end, '=');
+                const char *subst_beg = colon + 1;
+                const char *subst_end = lindex (subst_beg, end, '=');
                 if (subst_end == 0)
                   /* There is no = in sight.  Punt on the substitution
                      reference and treat this as a variable name containing
@@ -315,8 +313,8 @@ variable_expand_string (char *line, const char *string, long length)
                   colon = 0;
                 else
                   {
-                    replace_beg = subst_end + 1;
-                    replace_end = end;
+                    const char *replace_beg = subst_end + 1;
+                    const char *replace_end = end;
 
                     /* Extract the variable name before the colon
                        and look up that variable.  */

diff --git a/function.c b/function.c
index 431c0603740025e01909b5a2096467413100f1d2..9eabd73a313b3a4d0d035f23d7e7871626aa860b 100644 (file)
--- a/function.c
+++ b/function.c
@@ -1113,7 +1113,6 @@ func_sort (char *o, char **argv, const char *funcname UNUSED)
   int wordi;
   char *p;
   unsigned int len;
-  int i;
 
   /* Find the maximum number of words we'll have.  */
   t = argv[0];
@@ -1138,6 +1137,8 @@ func_sort (char *o, char **argv, const char *funcname UNUSED)
 
   if (wordi)
     {
+      int i;
+
       /* Now sort the list of words.  */
       qsort (words, wordi, sizeof (char *), alpha_compare);
 
@@ -1281,12 +1282,12 @@ static char *
 func_and (char *o, char **argv, const char *funcname UNUSED)
 {
   char *expansion;
-  int result;
 
   while (1)
     {
       const char *begp = *argv;
       const char *endp = begp + strlen (*argv) - 1;
+      int result;
 
       /* An empty condition is always false.  */
       strip_whitespace (&begp, &endp);

diff --git a/job.c b/job.c
index 01989055989abec14b411a3c2d69d46fb1315476..16c164ea775c007e3e57e1065e3e89f4ae3fac67 100644 (file)
--- a/job.c
+++ b/job.c
@@ -427,8 +427,8 @@ _is_unixy_shell (const char *path)
 int
 is_bourne_compatible_shell (const char *path)
 {
-  /* list of known unix (Bourne-like) shells */
-  const char *unix_shells[] = {
+  /* List of known POSIX (or POSIX-ish) shells.  */
+  static const char *unix_shells[] = {
     "sh",
     "bash",
     "ksh",
@@ -438,7 +438,7 @@ is_bourne_compatible_shell (const char *path)
     "dash",
     NULL
   };
-  unsigned i, len;
+  const char **s;
 
   /* find the rightmost '/' or '\\' */
   const char *name = strrchr (path, '/');
@@ -451,18 +451,18 @@ is_bourne_compatible_shell (const char *path)
   else if (!name)   /* name and p must be 0 */
     name = path;
 
-  if (*name == '/' || *name == '\\') name++;
+  if (*name == '/' || *name == '\\')
+    ++name;
 
   /* this should be able to deal with extensions on Windows-like systems */
-  for (i = 0; unix_shells[i] != NULL; i++)
+  for (s = unix_shells; *s != NULL; ++s)
     {
-      len = strlen (unix_shells[i]);
 #if defined(WINDOWS32) || defined(__MSDOS__)
-      if ((strncasecmp (name, unix_shells[i], len) == 0) &&
-          (strlen (name) >= len && STOP_SET (name[len], MAP_DOT|MAP_NUL)))
+      unsigned int len = strlen (*s);
+      if ((strlen (name) >= len && STOP_SET (name[len], MAP_DOT|MAP_NUL))
+          && strncasecmp (name, *s, len) == 0)
 #else
-      if ((strncmp (name, unix_shells[i], len) == 0) &&
-          (strlen (name) >= len && name[len] == '\0'))
+      if (strcmp (name, *s) == 0)
 #endif
         return 1; /* a known unix-style shell */
     }
@@ -992,7 +992,6 @@ reap_children (int block, int err)
 #ifdef WINDOWS32
           {
             HANDLE hPID;
-            int werr;
             HANDLE hcTID, hcPID;
             DWORD dwWaitStatus = 0;
             exit_code = 0;
@@ -1021,9 +1020,8 @@ reap_children (int block, int err)
             hPID = process_wait_for_any (block, &dwWaitStatus);
             if (hPID)
               {
-
                 /* was an error found on this process? */
-                werr = process_last_err (hPID);
+                int werr = process_last_err (hPID);
 
                 /* get exit data */
                 exit_code = process_exit_code (hPID);
@@ -2553,7 +2551,6 @@ exec_command (char **argv, char **envp)
 #ifdef WINDOWS32
   HANDLE hPID;
   HANDLE hWaitPID;
-  int err = 0;
   int exit_code = EXIT_FAILURE;
 
   /* make sure CreateProcess() has Path it needs */
@@ -2579,7 +2576,7 @@ exec_command (char **argv, char **envp)
   while (hWaitPID)
     {
       /* was an error found on this process? */
-      err = process_last_err (hWaitPID);
+      int err = process_last_err (hWaitPID);
 
       /* get exit data */
       exit_code = process_exit_code (hWaitPID);
@@ -2618,10 +2615,8 @@ exec_command (char **argv, char **envp)
   child_access ();
 
 # ifdef __EMX__
-
   /* Run the program.  */
   pid = spawnvpe (P_NOWAIT, argv[0], argv, envp);
-
   if (pid >= 0)
     return pid;
 
@@ -2630,7 +2625,6 @@ exec_command (char **argv, char **envp)
     errno = ENOEXEC;
 
 # else
-
   /* Run the program.  */
   environ = envp;
   execvp (argv[0], argv);

diff --git a/main.c b/main.c
index d6ea34d7d5740fb5fe151d7c2912a5ae1457facd..801365fa6e92ee04d34a7ad1eacfa0cbf0db0b5f 100644 (file)
--- a/main.c
+++ b/main.c
@@ -1232,11 +1232,10 @@ main (int argc, char **argv, char **envp)
       if (program == 0)
         {
           /* Extract program from full path */
-          int argv0_len;
           program = strrchr (argv[0], '\\');
           if (program)
             {
-              argv0_len = strlen (program);
+              int argv0_len = strlen (program);
               if (argv0_len > 4 && streq (&program[argv0_len - 4], ".exe"))
                 /* Remove .exe extension */
                 program[argv0_len - 4] = '\0';

diff --git a/misc.c b/misc.c
index d3a1da5284d0a638878c989b5a724dd1b371729a..8ae67b96eb0efb26260602f8465f5db700677fc5 100644 (file)
--- a/misc.c
+++ b/misc.c
@@ -50,9 +50,7 @@ alpha_compare (const void *v1, const void *v2)
 void
 collapse_continuations (char *line)
 {
-  register char *in, *out, *p;
-  register int backslash;
-  register unsigned int bs_write;
+  char *in, *out, *p;
 
   in = strchr (line, '\n');
   if (in == 0)
@@ -67,8 +65,8 @@ collapse_continuations (char *line)
       /* BS_WRITE gets the number of quoted backslashes at
          the end just before IN, and BACKSLASH gets nonzero
          if the next character is quoted.  */
-      backslash = 0;
-      bs_write = 0;
+      unsigned int backslash = 0;
+      unsigned int bs_write = 0;
       for (p = in - 1; p >= line && *p == '\\'; --p)
         {
           if (backslash)

diff --git a/read.c b/read.c
index 07bc8da99dc53f92b4e745fdf806e446b05c08ec..bc78fa5197e35b435226b4305dce7078876dd2fa 100644 (file)
--- a/read.c
+++ b/read.c
@@ -1297,14 +1297,13 @@ eval (struct ebuffer *ebuf, int set_default)
 
         if (set_default && default_goal_var->value[0] == '\0')
           {
-            const char *name;
             struct dep *d;
             struct nameseq *t = filenames;
 
             for (; t != 0; t = t->next)
               {
                 int reject = 0;
-                name = t->name;
+                const char *name = t->name;
 
                 /* We have nothing to do if this is an implicit rule. */
                 if (strchr (name, '%') != 0)
@@ -2981,7 +2980,6 @@ parse_file_seq (char **stringp, unsigned int size, int stopmap,
   /* tmp points to tmpbuf after the prefix, if any.
      tp is the end of the buffer. */
   static char *tmpbuf = NULL;
-  static int tmpbuf_len = 0;
 
   int cachep = NONE_SET (flags, PARSEFS_NOCACHE);
 
@@ -3009,6 +3007,7 @@ parse_file_seq (char **stringp, unsigned int size, int stopmap,
 
   /* Get enough temporary space to construct the largest possible target.  */
   {
+    static int tmpbuf_len = 0;
     int l = strlen (*stringp) + 1;
     if (l > tmpbuf_len)
       {

diff --git a/remake.c b/remake.c
index 713acfda5e0d9d2123cf37d59bef01f9aae6e4e5..138cdc686ec0e628f69f9eb43491f8bb46705200 100644 (file)
--- a/remake.c
+++ b/remake.c
@@ -1402,7 +1402,6 @@ f_mtime (struct file *file, int search)
          started.  So, turn off the intermediate bit so make doesn't
          delete it, since it didn't create it.  */
       if (mtime != NONEXISTENT_MTIME && file->command_state == cs_not_started
-          && file->command_state == cs_not_started
           && !file->tried_implicit && file->intermediate)
         file->intermediate = 0;
 

diff --git a/signame.c b/signame.c
index c8e45dace286d271103e8ae76c9db09ef6ddee76..59dc9498e8f10afc295ff570b45109ffe2e2c572 100644 (file)
--- a/signame.c
+++ b/signame.c
@@ -244,7 +244,7 @@ strsignal (int sig)
 # endif
 #endif
 
-  if (sig > 0 || sig < NSIG)
+  if (sig > 0 && sig < NSIG)
     return (char *) sys_siglist[sig];
 
   sprintf (buf, "Signal %d", sig);

diff --git a/vpath.c b/vpath.c
index 5b7ea937ce014375195fcac7366e7bb2373fc0ce..94956fa30c65b42913b899b148bd2288aef5725f 100644 (file)
--- a/vpath.c
+++ b/vpath.c
@@ -306,12 +306,13 @@ construct_vpath_list (char *pattern, char *dirpath)
 int
 gpath_search (const char *file, unsigned int len)
 {
-  const char **gp;
-
   if (gpaths && (len <= gpaths->maxlen))
-    for (gp = gpaths->searchpath; *gp != NULL; ++gp)
-      if (strneq (*gp, file, len) && (*gp)[len] == '\0')
-        return 1;
+    {
+      const char **gp;
+      for (gp = gpaths->searchpath; *gp != NULL; ++gp)
+        if (strneq (*gp, file, len) && (*gp)[len] == '\0')
+          return 1;
+    }
 
   return 0;
 }
@@ -334,7 +335,7 @@ selective_vpath_search (struct vpath *path, const char *file,
   const char **vpath = path->searchpath;
   unsigned int maxvpath = path->maxlen;
   unsigned int i;
-  unsigned int flen, vlen, name_dplen;
+  unsigned int flen, name_dplen;
   int exists = 0;
 
   /* Find out if *FILE is a target.
@@ -374,12 +375,10 @@ selective_vpath_search (struct vpath *path, const char *file,
   for (i = 0; vpath[i] != 0; ++i)
     {
       int exists_in_cache = 0;
-      char *p;
-
-      p = name;
+      char *p = name;
+      unsigned int vlen = strlen (vpath[i]);
 
       /* Put the next VPATH entry into NAME at P and increment P past it.  */
-      vlen = strlen (vpath[i]);
       memcpy (p, vpath[i], vlen);
       p += vlen;