Issue an error message when fts_read fails. Fixes bug #39324.rel-4-4-fixes

* find/ftsfind.c (find): when fts_read fails, issue an error
message, set the exit status to zero and stop.  Previously the
program would just stop (i.e. it failed to distinguish "done" from
"failed").
* find/find.1 (-exec): explain that on failure, some pending
command launches may not happen.  The Texinfo documentation
already pointed this out, so that didn't need to be changed.
* NEWS: Mention this bugfix.

7 files changed, 134 insertions, 67 deletions

diff --git a/ChangeLog b/ChangeLog
index 1d0a2153..94ec693e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2013-09-22  James Youngman  <jay@gnu.org>
+
+	Issue an error message when fts_read fails. Fixes bug #39324.
+	* find/ftsfind.c (find): when fts_read fails, issue an error
+	message, set the exit status to zero and stop.  Previously the
+	program would just stop (i.e. it failed to distinguish "done" from
+	"failed").
+	* find/find.1 (-exec): explain that on failure, some pending
+	command launches may not happen.  The Texinfo documentation
+	already pointed this out, so that didn't need to be changed.
+	* NEWS: Mention this bugfix.
+
 2011-04-02  James Youngman  <jay@gnu.org>
 
 	Use gnulib's parse-datetime module instead of getdate.
diff --git a/NEWS b/NEWS
index a57ca21f..131df02c 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,8 @@ GNU findutils NEWS - User visible changes.	-*- outline -*- (allout)
 
 ** Bug Fixes
 
+#39324: exits without error on OOM
+
 #28872: Mistake in "#safer" example in "Problems with -exec and
         filenames" section of the Texinfo manual.
 
diff --git a/build-aux/.cvsignore b/build-aux/.cvsignore
index b13a7cdf..c5ddfe3b 100644
--- a/build-aux/.cvsignore
+++ b/build-aux/.cvsignore
@@ -12,3 +12,6 @@ Makefile.in
 Makefile
 mkinstalldirs
 ylwrap
+arg-nonnull.h
+c++defs.h
+warn-on-use.h
diff --git a/build-aux/.gitignore b/build-aux/.gitignore
index ad7eca39..f35bf96b 100644
--- a/build-aux/.gitignore
+++ b/build-aux/.gitignore
@@ -11,3 +11,6 @@ Makefile.in
 mkinstalldirs
 ylwrap
 compile
+/arg-nonnull.h
+/c++defs.h
+/warn-on-use.h
diff --git a/doc/.cvsignore b/doc/.cvsignore
index 2568e31b..bf4307d8 100644
--- a/doc/.cvsignore
+++ b/doc/.cvsignore
@@ -30,3 +30,4 @@ versionmaint.texi
 fdl.texi
 gpl-3.0.texi
 regexprops-generic.texi
+parse-datetime.texi
diff --git a/find/find.1 b/find/find.1
index 8b67ae38..f5542467 100644
--- a/find/find.1
+++ b/find/find.1
@@ -1009,6 +1009,11 @@ command line is built in much the same way that
 .B xargs
 builds its command lines.  Only one instance of `{}' is allowed within
 the command.  The command is executed in the starting directory.
+If
+.B find
+encounters an error, this can sometimes cause an
+immediate exit, so some pending commands may not be run
+at all.
 
 .IP "\-execdir \fIcommand\fR ;"
 .IP "\-execdir \fIcommand\fR {} +"
@@ -1038,6 +1043,11 @@ appropriately-named file in a directory in which you will run
 The same applies to having entries in 
 .B $PATH
 which are empty or which are not absolute directory names.
+If
+.B find
+encounters an error, this can sometimes cause an
+immediate exit, so some pending commands may not be run
+at all.
 
 .IP "\-fls \fIfile\fR"
 True; like 
@@ -1977,6 +1987,16 @@ description, but if the return value is non-zero, you should not rely
 on the correctness of the results of 
 .BR find .
 
+When some error occurs,
+.B find
+may stop immeidately, without completing all the actions specified.
+For example, some starting points may not have been examined or some
+pending program invocations for
+.B \-exec ... {} +
+or
+.B \-execdir ... {} +
+may not have been performed.
+
 .SH "SEE ALSO"
 \fBlocate\fP(1), \fBlocatedb\fP(5), \fBupdatedb\fP(1), \fBxargs\fP(1),
 \fBchmod\fP(1), \fBfnmatch\fP(3), \fBregex\fP(7), \fBstat\fP(2),
diff --git a/find/ftsfind.c b/find/ftsfind.c
index 26392ce6..5c49ab4f 100644
--- a/find/ftsfind.c
+++ b/find/ftsfind.c
@@ -17,13 +17,13 @@
 */
 
 /* This file was written by James Youngman, based on find.c.
-   
+
    GNU find was written by Eric Decker <cire@cisco.com>,
    with enhancements by David MacKenzie <djm@gnu.org>,
    Jay Plett <jay@silence.princeton.nj.us>,
    and Tim Wood <axolotl!tim@toad.com>.
    The idea for -print0 and xargs -0 came from
-   Dan Bernstein <brnstnd@kramden.acf.nyu.edu>.  
+   Dan Bernstein <brnstnd@kramden.acf.nyu.edu>.
 */
 
 
@@ -116,15 +116,15 @@ static void left_dir(void)
 
 /*
  * Signal that we are now inside a directory pointed to by dir_fd.
- * The caller can't tell if this is the first time this happens, so 
- * we have to be careful not to call dup() more than once 
+ * The caller can't tell if this is the first time this happens, so
+ * we have to be careful not to call dup() more than once
  */
 static void inside_dir(int dir_fd)
 {
   if (ftsoptions & FTS_CWDFD)
     {
       assert (dir_fd == AT_FDCWD || dir_fd >= 0);
-      
+
       state.cwd_dir_fd = dir_fd;
       if (curr_fd < 0)
 	{
@@ -137,7 +137,7 @@ static void inside_dir(int dir_fd)
 	      curr_fd = dup(dir_fd);
 	      set_close_on_exec(curr_fd);
 	    }
-	  else 
+	  else
 	    {
 	      /* curr_fd is invalid, but dir_fd is also invalid.
 	       * This should not have happened.
@@ -148,7 +148,7 @@ static void inside_dir(int dir_fd)
     }
   else
     {
-      /* FTS_CWDFD is not in use.  We can always assume that 
+      /* FTS_CWDFD is not in use.  We can always assume that
        * AT_FDCWD refers to the directory we are currentl searching.
        *
        * Therefore there is nothing to do.
@@ -164,7 +164,7 @@ static void init_mounted_dev_list(void);
 
 /* We have encountered an error which should affect the exit status.
  * This is normally used to change the exit status from 0 to 1.
- * However, if the exit status is already 2 for example, we don't want to 
+ * However, if the exit status is already 2 for example, we don't want to
  * reduce it to 1.
  */
 static void
@@ -208,7 +208,7 @@ static void
 visit(FTS *p, FTSENT *ent, struct stat *pstat)
 {
   struct predicate *eval_tree;
-  
+
   state.curdepth = ent->fts_level;
   state.have_stat = (ent->fts_info != FTS_NS) && (ent->fts_info != FTS_NSOK);
   state.rel_pathname = ent->fts_accpath;
@@ -236,7 +236,7 @@ partial_quotearg_n(int n, char *s, size_t len, enum quoting_style style)
     {
       char saved;
       const char *result;
-      
+
       saved = s[len];
       s[len] = 0;
       result = quotearg_n_style(n, style, s);
@@ -246,14 +246,14 @@ partial_quotearg_n(int n, char *s, size_t len, enum quoting_style style)
 }
 
 
-/* We've detected a file system loop.   This is caused by one of 
+/* We've detected a file system loop.   This is caused by one of
  * two things:
  *
- * 1. Option -L is in effect and we've hit a symbolic link that 
- *    points to an ancestor.  This is harmless.  We won't traverse the 
+ * 1. Option -L is in effect and we've hit a symbolic link that
+ *    points to an ancestor.  This is harmless.  We won't traverse the
  *    symbolic link.
  *
- * 2. We have hit a real cycle in the directory hierarchy.  In this 
+ * 2. We have hit a real cycle in the directory hierarchy.  In this
  *    case, we issue a diagnostic message (POSIX requires this) and we
  *    skip that directory entry.
  */
@@ -287,13 +287,13 @@ issue_loop_warning(FTSENT * ent)
     }
 }
 
-/* 
- * Return true if NAME corresponds to a file which forms part of a 
- * symbolic link loop.  The command 
- *      rm -f a b; ln -s a b; ln -s b a 
+/*
+ * Return true if NAME corresponds to a file which forms part of a
+ * symbolic link loop.  The command
+ *      rm -f a b; ln -s a b; ln -s b a
  * produces such a loop.
  */
-static boolean 
+static boolean
 symlink_loop(const char *name)
 {
   struct stat stbuf;
@@ -305,7 +305,7 @@ symlink_loop(const char *name)
   return (0 != rv) && (ELOOP == errno);
 }
 
-  
+
 static void
 show_outstanding_execdirs(FILE *fp)
 {
@@ -319,7 +319,7 @@ show_outstanding_execdirs(FILE *fp)
       while (p)
 	{
 	  const char *pfx;
-	  
+
 	  if (pred_is(p, pred_execdir))
 	    pfx = "-execdir";
 	  else if (pred_is(p, pred_okdir))
@@ -331,7 +331,7 @@ show_outstanding_execdirs(FILE *fp)
 	      int i;
 	      const struct exec_val *execp = &p->args.exec_vec;
 	      ++seen;
-	      
+
 	      fprintf(fp, "%s ", pfx);
 	      if (execp->multiple)
 		fprintf(fp, "multiple ");
@@ -362,7 +362,7 @@ consider_visiting(FTS *p, FTSENT *ent)
   struct stat statbuf;
   mode_t mode;
   int ignore, isdir;
-  
+
   if (options.debug_options & DebugSearch)
     fprintf(stderr,
 	    "consider_visiting: fts_info=%-6s, fts_level=%2d, prev_depth=%d "
@@ -371,7 +371,7 @@ consider_visiting(FTS *p, FTSENT *ent)
             (int)ent->fts_level, prev_depth,
 	    quotearg_n_style(0, options.err_quoting_style, ent->fts_path),
 	    quotearg_n_style(1, options.err_quoting_style, ent->fts_accpath));
-  
+
   if (ent->fts_info == FTS_DP)
     {
       left_dir();
@@ -383,7 +383,7 @@ consider_visiting(FTS *p, FTSENT *ent)
   inside_dir(p->fts_cwd_fd);
   prev_depth = ent->fts_level;
 
-  
+
   /* Cope with various error conditions. */
   if (ent->fts_info == FTS_ERR
       || ent->fts_info == FTS_DNR)
@@ -404,8 +404,8 @@ consider_visiting(FTS *p, FTSENT *ent)
       /* fts_read() claims that ent->fts_accpath is a broken symbolic
        * link.  That would be fine, but if this is part of a symbolic
        * link loop, we diagnose the problem and also ensure that the
-       * eventual return value is nonzero.   Note that while the path 
-       * we stat is local (fts_accpath), we print the full path name 
+       * eventual return value is nonzero.   Note that while the path
+       * we stat is local (fts_accpath), we print the full path name
        * of the file (fts_path) in the error message.
        */
       if (symlink_loop(ent->fts_accpath))
@@ -439,7 +439,7 @@ consider_visiting(FTS *p, FTSENT *ent)
 	    }
 	}
     }
-  
+
   /* Cope with the usual cases. */
   if (ent->fts_info == FTS_NSOK
       || ent->fts_info == FTS_NS /* e.g. symlink loop */)
@@ -454,7 +454,7 @@ consider_visiting(FTS *p, FTSENT *ent)
       state.have_type = true;
       statbuf = *(ent->fts_statp);
       state.type = mode = statbuf.st_mode;
-      
+
       if (00000 == mode)
 	{
 	  /* Savannah bug #16378. */
@@ -492,20 +492,20 @@ consider_visiting(FTS *p, FTSENT *ent)
       if (ent->fts_level >= options.maxdepth)
 	{
 	  fts_set(p, ent, FTS_SKIP); /* descend no further */
-	  
-	  if (ent->fts_level > options.maxdepth) 
+
+	  if (ent->fts_level > options.maxdepth)
 	    ignore = 1;		/* don't even look at this one */
 	}
     }
 
   if ( (ent->fts_info == FTS_D) && !options.do_dir_first )
     {
-      /* this is the preorder visit, but user said -depth */ 
+      /* this is the preorder visit, but user said -depth */
       ignore = 1;
     }
   else if ( (ent->fts_info == FTS_DP) && options.do_dir_first )
     {
-      /* this is the postorder visit, but user didn't say -depth */ 
+      /* this is the postorder visit, but user didn't say -depth */
       ignore = 1;
     }
   else if (ent->fts_level < options.mindepth)
@@ -527,30 +527,30 @@ consider_visiting(FTS *p, FTSENT *ent)
 
 
 
-static void
+static boolean
 find(char *arg)
 {
   char * arglist[2];
   FTS *p;
   FTSENT *ent;
-  
+
 
   state.starting_path_length = strlen(arg);
   inside_dir(AT_FDCWD);
 
   arglist[0] = arg;
   arglist[1] = NULL;
-  
+
   switch (options.symlink_handling)
     {
     case SYMLINK_ALWAYS_DEREF:
       ftsoptions |= FTS_COMFOLLOW|FTS_LOGICAL;
       break;
-	  
+
     case SYMLINK_DEREF_ARGSONLY:
       ftsoptions |= FTS_COMFOLLOW|FTS_PHYSICAL;
       break;
-	  
+
     case SYMLINK_NEVER_DEREF:
       ftsoptions |= FTS_PHYSICAL;
       break;
@@ -558,18 +558,19 @@ find(char *arg)
 
   if (options.stay_on_filesystem)
     ftsoptions |= FTS_XDEV;
-      
+
   p = fts_open(arglist, ftsoptions, NULL);
   if (NULL == p)
     {
       error (0, errno, _("cannot search %s"),
 	     safely_quote_err_filename(0, arg));
+      return false;
     }
   else
     {
       int level = INT_MIN;
 
-      while ( (ent=fts_read(p)) != NULL )
+      while ( (errno=0, ent=fts_read(p)) != NULL )
 	{
 	  if (state.execdirs_outstanding)
 	    {
@@ -593,13 +594,35 @@ find(char *arg)
 	  state.type = 0;
 	  consider_visiting(p, ent);
 	}
-      fts_close(p);
+      /* fts_read returned NULL; distinguish between "finished" and "error". */
+      if (errno)
+	{
+	  error (0, errno,
+		 "failed to read file names from file system at or below %s",
+		 safely_quote_err_filename (0, arg));
+	  error_severity (EXIT_FAILURE);
+	  return false;
+	}
+
+      if (0 != fts_close (p))
+	{
+	  /* Here we break the abstraction of fts_close a bit, because we
+	   * are going to skip the rest of the start points, and return with
+	   * nonzero exit status.  Hence we need to issue a diagnostic on
+	   * stderr. */
+	  error (0, errno,
+		 _("failed to restore working directory after searching %s"),
+		 arg);
+	  error_severity (EXIT_FAILURE);
+	  return false;
+	}
       p = NULL;
     }
+  return true;
 }
 
 
-static void 
+static boolean
 process_all_startpoints(int argc, char *argv[])
 {
   int i;
@@ -607,21 +630,23 @@ process_all_startpoints(int argc, char *argv[])
   /* figure out how many start points there are */
   for (i = 0; i < argc && !looks_like_expression(argv[i], true); i++)
     {
-      state.starting_path_length = strlen(argv[i]); /* TODO: is this redundant? */
-      find(argv[i]);
+      state.starting_path_length = strlen (argv[i]); /* TODO: is this redundant? */
+      if (!find (argv[i]))
+	return false;
     }
 
   if (i == 0)
     {
-      /* 
-       * We use a temporary variable here because some actions modify 
-       * the path temporarily.  Hence if we use a string constant, 
-       * we get a coredump.  The best example of this is if we say 
+      /*
+       * We use a temporary variable here because some actions modify
+       * the path temporarily.  Hence if we use a string constant,
+       * we get a coredump.  The best example of this is if we say
        * "find -printf %H" (note, not "find . -printf %H").
        */
       char defaultpath[2] = ".";
-      find(defaultpath);
+      return find (defaultpath);
     }
+  return true;
 }
 
 
@@ -644,7 +669,7 @@ main (int argc, char **argv)
    * check_nofollow() needs to be executed in the POSIX locale.
    */
   set_option_defaults(&options);
-  
+
 #ifdef HAVE_SETLOCALE
   setlocale (LC_ALL, "");
 #endif
@@ -653,11 +678,11 @@ main (int argc, char **argv)
   textdomain (PACKAGE);
   atexit (close_stdout);
 
-  /* Check for -P, -H or -L options.  Also -D and -O, which are 
+  /* Check for -P, -H or -L options.  Also -D and -O, which are
    * both GNU extensions.
    */
   end_of_leading_options = process_leading_options(argc, argv);
-  
+
   if (options.debug_options & DebugStat)
     options.xstat = debug_stat;
 
@@ -666,20 +691,20 @@ main (int argc, char **argv)
 #endif /* DEBUG */
 
 
-  /* We are now processing the part of the "find" command line 
+  /* We are now processing the part of the "find" command line
    * after the -H/-L options (if any).
    */
   eval_tree = build_expression_tree(argc, argv, end_of_leading_options);
 
-  /* safely_chdir() needs to check that it has ended up in the right place. 
-   * To avoid bailing out when something gets automounted, it checks if 
+  /* safely_chdir() needs to check that it has ended up in the right place.
+   * To avoid bailing out when something gets automounted, it checks if
    * the target directory appears to have had a directory mounted on it as
-   * we chdir()ed.  The problem with this is that in order to notice that 
+   * we chdir()ed.  The problem with this is that in order to notice that
    * a file system was mounted, we would need to lstat() all the mount points.
    * That strategy loses if our machine is a client of a dead NFS server.
    *
-   * Hence if safely_chdir() and wd_sanity_check() can manage without needing 
-   * to know the mounted device list, we do that.  
+   * Hence if safely_chdir() and wd_sanity_check() can manage without needing
+   * to know the mounted device list, we do that.
    */
   if (!options.open_nofollow_available)
     {
@@ -687,16 +712,17 @@ main (int argc, char **argv)
       init_mounted_dev_list();
 #endif
     }
-  
 
-  process_all_startpoints(argc-end_of_leading_options, argv+end_of_leading_options);
-  
-  /* If "-exec ... {} +" has been used, there may be some 
-   * partially-full command lines which have been built, 
-   * but which are not yet complete.   Execute those now.
-   */
-  show_success_rates(eval_tree);
-  cleanup();
+
+  if (process_all_startpoints(argc-end_of_leading_options, argv+end_of_leading_options))
+    {
+      /* If "-exec ... {} +" has been used, there may be some
+       * partially-full command lines which have been built,
+       * but which are not yet complete.   Execute those now.
+       */
+      show_success_rates(eval_tree);
+      cleanup();
+    }
   return state.exit_status;
 }