diff --git a/ChangeLog b/ChangeLog index 8454b08c..608f8702 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,25 @@ 2013-07-21 Paul Smith + 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 . diff --git a/arscan.c b/arscan.c index afdb6016..2b3cd5df 100644 --- 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 5681275e..0c93ac9c 100644 --- 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 4eabcc64..ba04e484 100644 --- 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 431c0603..9eabd73a 100644 --- 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 01989055..16c164ea 100644 --- 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 d6ea34d7..801365fa 100644 --- 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 d3a1da52..8ae67b96 100644 --- 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 07bc8da9..bc78fa51 100644 --- 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 713acfda..138cdc68 100644 --- 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 c8e45dac..59dc9498 100644 --- 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 5b7ea937..94956fa3 100644 --- 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;