Resolve issues discovered by static code analysis.

* maintMakefile: Add a rule to submit code for analysis.
* configure.ac: Check for availability of the umask() function.
* output.c (output_tmpfd, output_tmpfile): Use umask on temp files.
* makeint.h (PATH_VAR): Reserve an extra character for nul bytes.
* function.c (func_error): Initialize buffer to empty string.
* job.c (child_execute_job): Verify validity of fdin.
* main.c (main): Simplify code for makefile updating algorithm.
* arscan.c (ar_scan): Verify member name length before reading.
* read.c (readline): Cast pointer arithmetic to avoid warnings.
* remake.c (update_file): Remove unreachable code.
(name_mtime): Verify symlink name length.

arscan.c

@@ -417,16 +417,14 @@ ar_scan (const char *archive, ar_member_func_t function, const void *arg)
   int desc = open (archive, O_RDONLY, 0);
   if (desc < 0)
     return -1;
+
 #ifdef SARMAG
   {
     char buf[SARMAG];
     int nread;
     EINTRLOOP (nread, read (desc, buf, SARMAG));
     if (nread != SARMAG || memcmp (buf, ARMAG, SARMAG))
-      {
+      goto invalid;
-        (void) close (desc);
-        return -2;
-      }
   }
 #else
 #ifdef AIAMAG
@@ -434,10 +432,8 @@ ar_scan (const char *archive, ar_member_func_t function, const void *arg)
     int nread;
     EINTRLOOP (nread, read (desc, &fl_header, FL_HSZ));
     if (nread != FL_HSZ)
-      {
+      goto invalid;
-        (void) close (desc);
+
-        return -2;
-      }
 #ifdef AIAMAGBIG
     /* If this is a "big" archive, then set the flag and
        re-read the header into the "big" structure. */
@@ -450,27 +446,18 @@ ar_scan (const char *archive, ar_member_func_t function, const void *arg)
         /* seek back to beginning of archive */
         EINTRLOOP (o, lseek (desc, 0, 0));
         if (o < 0)
-          {
+          goto invalid;
-            (void) close (desc);
-            return -2;
-          }
 
         /* re-read the header into the "big" structure */
         EINTRLOOP (nread, read (desc, &fl_header_big, FL_HSZ_BIG));
         if (nread != FL_HSZ_BIG)
-          {
+          goto invalid;
-            (void) close (desc);
-            return -2;
-          }
       }
     else
 #endif
        /* Check to make sure this is a "normal" archive. */
       if (memcmp (fl_header.fl_magic, AIAMAG, SAIAMAG))
-        {
+        goto invalid;
-          (void) close (desc);
-          return -2;
-        }
   }
 #else
   {
@@ -482,10 +469,7 @@ ar_scan (const char *archive, ar_member_func_t function, const void *arg)
     int nread;
     EINTRLOOP (nread, read (desc, &buf, sizeof (buf)));
     if (nread != sizeof (buf) || buf != ARMAG)
-      {
+      goto invalid;
-        (void) close (desc);
-        return -2;
-      }
   }
 #endif
 #endif
@@ -535,13 +519,15 @@ ar_scan (const char *archive, ar_member_func_t function, const void *arg)
         struct ar_hdr_big member_header_big;
 #endif
 #ifdef AIAMAG
-        char name[256];
+# define ARNAME_MAX 255
+        char name[ARNAME_MAX + 1];
         int name_len;
         long int dateval;
         int uidval, gidval;
         long int data_offset;
 #else
-        char namebuf[sizeof member_header.ar_name + 1];
+# define ARNAME_MAX (int)sizeof(member_header.ar_name)
+        char namebuf[ARNAME_MAX + 1];
         char *name;
         int is_namemap;         /* Nonzero if this entry maps long names.  */
         int long_name = 0;
@@ -553,10 +539,7 @@ ar_scan (const char *archive, ar_member_func_t function, const void *arg)
 
         EINTRLOOP (o, lseek (desc, member_offset, 0));
         if (o < 0)
-          {
+          goto invalid;
-            (void) close (desc);
-            return -2;
-          }
 
 #ifdef AIAMAG
 #define       AR_MEMHDR_SZ(x) (sizeof(x) - sizeof (x._ar_name))
@@ -568,21 +551,17 @@ ar_scan (const char *archive, ar_member_func_t function, const void *arg)
                                     AR_MEMHDR_SZ(member_header_big)));
 
             if (nread != AR_MEMHDR_SZ(member_header_big))
-              {
+              goto invalid;
-                (void) close (desc);
-                return -2;
-              }
 
             sscanf (member_header_big.ar_namlen, "%4d", &name_len);
+            if (name_len < 1 || name_len > ARNAME_MAX)
+              goto invalid;
+
             EINTRLOOP (nread, read (desc, name, name_len));
-
             if (nread != name_len)
-              {
+              goto invalid;
-                (void) close (desc);
-                return -2;
-              }
 
-            name[name_len] = 0;
+            name[name_len] = '\0';
 
             sscanf (member_header_big.ar_date, "%12ld", &dateval);
             sscanf (member_header_big.ar_uid, "%12d", &uidval);
@@ -600,21 +579,17 @@ ar_scan (const char *archive, ar_member_func_t function, const void *arg)
                                     AR_MEMHDR_SZ(member_header)));
 
             if (nread != AR_MEMHDR_SZ(member_header))
-              {
+              goto invalid;
-                (void) close (desc);
-                return -2;
-              }
 
             sscanf (member_header.ar_namlen, "%4d", &name_len);
+            if (name_len < 1 || name_len > ARNAME_MAX)
+              goto invalid;
+
             EINTRLOOP (nread, read (desc, name, name_len));
-
             if (nread != name_len)
-              {
+              goto invalid;
-                (void) close (desc);
-                return -2;
-              }
 
-            name[name_len] = 0;
+            name[name_len] = '\0';
 
             sscanf (member_header.ar_date, "%12ld", &dateval);
             sscanf (member_header.ar_uid, "%12d", &uidval);
@@ -656,10 +631,7 @@ ar_scan (const char *archive, ar_member_func_t function, const void *arg)
                )
 #endif
             )
-          {
+          goto invalid;
-            (void) close (desc);
-            return -2;
-          }
 
         name = namebuf;
         memcpy (name, member_header.ar_name, sizeof member_header.ar_name);
@@ -679,6 +651,7 @@ ar_scan (const char *archive, ar_member_func_t function, const void *arg)
           is_namemap = (!strcmp (name, "//")
                         || !strcmp (name, "ARFILENAMES/"));
 #endif  /* Not AIAMAG. */
+
           /* On some systems, there is a slash after each member name.  */
           if (*p == '/')
             *p = '\0';
@@ -693,23 +666,27 @@ ar_scan (const char *archive, ar_member_func_t function, const void *arg)
               && (name[0] == ' ' || name[0] == '/')
               && namemap != 0)
             {
-              name = namemap + atoi (name + 1);
+              int name_off = atoi (name + 1);
+              if (name_off < 1 || name_off > ARNAME_MAX)
+                goto invalid;
+
+              name = namemap + name_off;
               long_name = 1;
             }
           else if (name[0] == '#'
                    && name[1] == '1'
                    && name[2] == '/')
             {
-              int namesize = atoi (name + 3);
+              int name_len = atoi (name + 3);
+              if (name_len < 1 || name_len > ARNAME_MAX)
+                goto invalid;
 
-              name = alloca (namesize + 1);
+              name = alloca (name_len + 1);
-              EINTRLOOP (nread, read (desc, name, namesize));
+              EINTRLOOP (nread, read (desc, name, name_len));
-              if (nread != namesize)
+              if (nread != name_len)
-                {
+                goto invalid;
-                  close (desc);
+
-                  return -2;
+              name[name_len] = '\0';
-                }
-              name[namesize] = '\0';
 
               long_name = 1;
             }
@@ -759,10 +736,7 @@ ar_scan (const char *archive, ar_member_func_t function, const void *arg)
           sscanf (member_header.ar_nxtmem, "%12ld", &member_offset);
 
         if (lseek (desc, member_offset, 0) != member_offset)
-          {
+          goto invalid;
-            (void) close (desc);
-            return -2;
-          }
 #else
 
         /* If this member maps archive names, we must read it in.  The
@@ -776,10 +750,7 @@ ar_scan (const char *archive, ar_member_func_t function, const void *arg)
             namemap = alloca (eltsize);
             EINTRLOOP (nread, read (desc, namemap, eltsize));
             if (nread != eltsize)
-              {
+              goto invalid;
-                (void) close (desc);
-                return -2;
-              }
 
             /* The names are separated by newlines.  Some formats have
                a trailing slash.  Null terminate the strings for
@@ -807,6 +778,10 @@ ar_scan (const char *archive, ar_member_func_t function, const void *arg)
 
   close (desc);
   return 0;
+
+ invalid:
+  close (desc);
+  return -2;
 }
 #endif /* !VMS */
 

configure.ac

@@ -132,7 +132,7 @@ AS_IF([test "$ac_cv_func_gettimeofday" = yes],
             [Define to 1 if you have a standard gettimeofday function])
 ])
 
-AC_CHECK_FUNCS([strdup strndup mkstemp mktemp fdopen fileno \
+AC_CHECK_FUNCS([strdup strndup umask mkstemp mktemp fdopen fileno \
                 dup dup2 getcwd realpath sigsetmask sigaction \
                 getgroups seteuid setegid setlinebuf setreuid setregid \
                 getrlimit setrlimit setvbuf pipe strerror strsignal \

function.c

@@ -1120,6 +1120,7 @@ func_error (char *o, char **argv, const char *funcname)
     len += strlen (*argvp) + 2;
 
   p = msg = alloca (len + 1);
+  msg[0] = '\0';
 
   for (argvp=argv; argvp[1] != 0; ++argvp)
     {

job.c

@@ -2151,7 +2151,7 @@ child_execute_job (struct output *out, int good_stdin, char **argv, char **envp)
 
   /* For any redirected FD, dup2() it to the standard FD.
      They are all marked close-on-exec already.  */
-  if (fdin != FD_STDIN)
+  if (fdin >= 0 && fdin != FD_STDIN)
     EINTRLOOP (r, dup2 (fdin, FD_STDIN));
   if (fdout != FD_STDOUT)
     EINTRLOOP (r, dup2 (fdout, FD_STDOUT));

main.c

@@ -2172,47 +2172,43 @@ main (int argc, char **argv, char **envp)
 
       DB (DB_BASIC, (_("Updating makefiles....\n")));
 
-      /* Remove any makefiles we don't want to try to update.
+      /* Remove any makefiles we don't want to try to update.  Record the
-         Also record the current modtimes so we can compare them later.  */
+         current modtimes of the others so we can compare them later.  */
       {
         register struct goaldep *d, *last;
         last = 0;
         d = read_files;
         while (d != 0)
           {
-            struct file *f = d->file;
+            struct file *f;
-            if (f->double_colon)
+            for (f = d->file->double_colon; f != NULL; f = f->prev)
-              for (f = f->double_colon; f != NULL; f = f->prev)
+              if (f->deps == 0 && f->cmds != 0)
-                {
+                break;
-                  if (f->deps == 0 && f->cmds != 0)
-                    {
-                      /* This makefile is a :: target with commands, but
-                         no dependencies.  So, it will always be remade.
-                         This might well cause an infinite loop, so don't
-                         try to remake it.  (This will only happen if
-                         your makefiles are written exceptionally
-                         stupidly; but if you work for Athena, that's how
-                         you write your makefiles.)  */
 
-                      DB (DB_VERBOSE,
+            if (f)
-                          (_("Makefile '%s' might loop; not remaking it.\n"),
+              {
-                           f->name));
+                /* This makefile is a :: target with commands, but no
+                   dependencies.  So, it will always be remade.  This might
+                   well cause an infinite loop, so don't try to remake it.
+                   (This will only happen if your makefiles are written
+                   exceptionally stupidly; but if you work for Athena, that's
+                   how you write your makefiles.)  */
 
-                      if (last == 0)
+                DB (DB_VERBOSE,
-                        read_files = d->next;
+                    (_("Makefile '%s' might loop; not remaking it.\n"),
-                      else
+                     f->name));
-                        last->next = d->next;
 
-                      /* Free the storage.  */
+                if (last)
-                      free_goaldep (d);
+                  last->next = d->next;
+                else
+                  read_files = d->next;
 
-                      d = last == 0 ? read_files : last->next;
+                /* Free the storage.  */
+                free_goaldep (d);
 
-                      break;
+                d = last ? last->next : read_files;
-                    }
+              }
-                }
+            else
-
-            if (f == NULL || !f->double_colon)
               {
                 makefile_mtimes = xrealloc (makefile_mtimes,
                                             (mm_idx+1)

maintMakefile

@@ -118,7 +118,6 @@ git-very-clean: git-clean
 	-$(GIT) clean -fd
 
 
-
 ## ---------------------- ##
 ## Generating ChangeLog.  ##
 ## ---------------------- ##
@@ -332,6 +331,55 @@ gendocs: update-gnuweb update-makeweb
 	    && echo '- cvs commit' \
 	    && echo '- cvs tag make-$(subst .,-,$(VERSION))'
 
+
+## --------------------------------------------- ##
+## Submitting Coverity cov-build results to Scan ##
+## --------------------------------------------- ##
+
+# Note you must have set COVERITY_TOKEN and COVERITY_EMAIL properly
+# to submit results.  COVERITY_PATH can be set to the root of the
+# cov-build tools if it's not already on your PATH.
+
+COV_BUILD_FILE := cov-build.tgz
+
+.PHONY: cov-build cov-submit
+
+cov-build: $(COV_BUILD_FILE)
+
+$(COV_BUILD_FILE): $(filter %.c %.h,$(DISTFILES))
+	$(MAKE) distdir
+	@echo "Running Coverity cov-build"
+	rm -rf '$(distdir)'/_build
+	mkdir '$(distdir)'/_build
+	cd '$(distdir)'/_build \
+	    && ../configure --srcdir=.. \
+		$(AM_DISTCHECK_CONFIGURE_FLAGS) $(DISTCHECK_CONFIGURE_FLAGS) \
+		CFLAGS='$(AM_CFLAGS)'
+	PATH="$${COVERITY_PATH:+$$COVERITY_PATH/bin:}$$PATH"; \
+	    cd '$(distdir)'/_build \
+		&& cov-build --dir cov-int ./build.sh
+	rm -f '$@'
+	(cd '$(distdir)'/_build && tar czf - cov-int) > '$@'
+
+cov-submit: $(COV_BUILD_FILE)-submitted
+
+$(COV_BUILD_FILE)-submitted: $(COV_BUILD_FILE)
+	@[ -n "$(COVERITY_TOKEN)" ] || { echo 'COVERITY_TOKEN not set'; exit 1; }
+	@[ -n "$(COVERITY_EMAIL)" ] || { echo 'COVERITY_EMAIL not set'; exit 1; }
+	rm -f '$@'
+	case '$(VERSION)' in \
+	    (*.*.9*) type="daily build"; ext=".$$(date +%Y%m%d)" ;; \
+	    (*)      type="release"; ext= ;; \
+	esac; \
+	curl --form token='$(COVERITY_TOKEN)' \
+	     --form email='$(COVERITY_EMAIL)' \
+	     --form file='@$<' \
+	     --form version="$(VERSION)$$ext" \
+	     --form description="GNU make $$type" \
+	     'https://scan.coverity.com/builds?project=gmake'
+	cp '$<' '$@'
+
+
 ## ------------------------- ##
 ## Make release targets.     ##
 ## ------------------------- ##
@@ -344,6 +392,7 @@ tag-release:
 	esac; \
 	$(GIT) tag -m "GNU Make release$$message $(VERSION)" -u '$(GPG_FINGERPRINT)' '$(VERSION)'
 
+
 ## ------------------------- ##
 ## GNU FTP upload artifacts. ##
 ## ------------------------- ##

makeint.h

@@ -156,11 +156,11 @@ extern int errno;
 
 #ifdef  PATH_MAX
 # define GET_PATH_MAX   PATH_MAX
-# define PATH_VAR(var)  char var[PATH_MAX]
+# define PATH_VAR(var)  char var[PATH_MAX+1]
 #else
 # define NEED_GET_PATH_MAX 1
 # define GET_PATH_MAX   (get_path_max ())
-# define PATH_VAR(var)  char *var = alloca (GET_PATH_MAX)
+# define PATH_VAR(var)  char *var = alloca (GET_PATH_MAX+1)
 unsigned int get_path_max (void);
 #endif
 

output.c

@@ -52,6 +52,14 @@ unsigned int stdio_traced = 0;
 # define STREAM_OK(_s) 1
 #endif
 
+#if defined(HAVE_UMASK)
+# define UMASK(_m)  umask (_m)
+# define MODE_T     mode_t
+#else
+# define UMASK(_m)  0
+# define MODE_T     int
+#endif
+
 /* Write a string to the current STDOUT or STDERR.  */
 static void
 _outputs (struct output *out, int is_err, const char *msg)
@@ -160,7 +168,10 @@ set_append_mode (int fd)
 #if defined(F_GETFL) && defined(F_SETFL) && defined(O_APPEND)
   int flags = fcntl (fd, F_GETFL, 0);
   if (flags >= 0)
-    fcntl (fd, F_SETFL, flags | O_APPEND);
+    {
+      int r;
+      EINTRLOOP(r, fcntl (fd, F_SETFL, flags | O_APPEND));
+    }
 #endif
 }
 
@@ -285,6 +296,7 @@ release_semaphore (void *sem)
 int
 output_tmpfd (void)
 {
+  MODE_T mask = UMASK (0077);
   int fd = -1;
   FILE *tfile = tmpfile ();
 
@@ -300,6 +312,8 @@ output_tmpfd (void)
 
   set_append_mode (fd);
 
+  UMASK (mask);
+
   return fd;
 }
 
@@ -414,11 +428,15 @@ char *mktemp (char *template);
 FILE *
 output_tmpfile (char **name, const char *template)
 {
+  FILE *file;
 #ifdef HAVE_FDOPEN
   int fd;
 #endif
 
-#if defined HAVE_MKSTEMP || defined HAVE_MKTEMP
+  /* Preserve the current umask, and set a restrictive one for temp files.  */
+  MODE_T mask = UMASK (0077);
+
+#if defined(HAVE_MKSTEMP) || defined(HAVE_MKTEMP)
 # define TEMPLATE_LEN   strlen (template)
 #else
 # define TEMPLATE_LEN   L_tmpnam
@@ -426,12 +444,13 @@ output_tmpfile (char **name, const char *template)
   *name = xmalloc (TEMPLATE_LEN + 1);
   strcpy (*name, template);
 
-#if defined HAVE_MKSTEMP && defined HAVE_FDOPEN
+#if defined(HAVE_MKSTEMP) && defined(HAVE_FDOPEN)
   /* It's safest to use mkstemp(), if we can.  */
-  fd = mkstemp (*name);
+  EINTRLOOP (fd, mkstemp (*name));
   if (fd == -1)
-    return 0;
+    file = NULL;
-  return fdopen (fd, "w");
+  else
+    file = fdopen (fd, "w");
 #else
 # ifdef HAVE_MKTEMP
   (void) mktemp (*name);
@@ -444,12 +463,16 @@ output_tmpfile (char **name, const char *template)
   EINTRLOOP (fd, open (*name, O_CREAT|O_EXCL|O_WRONLY, 0600));
   if (fd == -1)
     return 0;
-  return fdopen (fd, "w");
+  file = fdopen (fd, "w");
 # else
   /* Not secure, but what can we do?  */
-  return fopen (*name, "w");
+  file = fopen (*name, "w");
 # endif
 #endif
+
+  UMASK (mask);
+
+  return file;
 }
 
 

read.c

@@ -2524,7 +2524,7 @@ readline (struct ebuffer *ebuf)
   end = p + ebuf->size;
   *p = '\0';
 
-  while (fgets (p, end - p, ebuf->fp) != 0)
+  while (fgets (p, (int)(end - p), ebuf->fp) != 0)
     {
       char *p2;
       unsigned long len;

remake.c

@@ -353,23 +353,6 @@ update_file (struct file *file, unsigned int depth)
         status = new;
     }
 
-  /* Process the remaining rules in the double colon chain so they're marked
-     considered.  Start their prerequisites, too.  */
-  if (file->double_colon)
-    for (; f != 0 ; f = f->prev)
-      {
-        struct dep *d;
-
-        f->considered = considered;
-
-        for (d = f->deps; d != 0; d = d->next)
-          {
-            enum update_status new = update_file (d->file, depth + 1);
-            if (new > status)
-              status = new;
-          }
-      }
-
   return status;
 }
 
@@ -1510,7 +1493,7 @@ name_mtime (const char *name)
 #ifndef S_ISLNK
 # define S_ISLNK(_m)     (((_m)&S_IFMT)==S_IFLNK)
 #endif
-  if (check_symlink_flag)
+  if (check_symlink_flag && strlen (name) <= GET_PATH_MAX)
     {
       PATH_VAR (lpath);