On Mon, Apr 5, 2010 at 3:55 PM, Kamil Dudka <[email protected]> wrote:
> Hello,
>
> Sami Farin (CC'd) discovered some flaws in recent findutils and reported
> them at Red Hat bugzilla:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=579476#c0
> https://bugzilla.redhat.com/show_bug.cgi?id=579476#c2
>
> Here are proposed patches:
> https://bugzilla.redhat.com/attachment.cgi?id=404487
> https://bugzilla.redhat.com/attachment.cgi?id=404490

First of all, thank you for the really fast testing turnaround!

This is a known problem, Savannah bug #27213.   Yesterday, I mailed
patches to the list which should fix this.   See
<http://savannah.gnu.org/bugs/?27213> for information about the bug.

I attach a set of patches.  They're the same as the patches I emailed
yesterday, but rebased against current head.

James.
From f4966db137283dc090e189f3e7e4fdbbe153726b Mon Sep 17 00:00:00 2001
From: Martin von Gagern <[email protected]>
Date: Wed, 26 Aug 2009 11:20:02 +0200
Subject: [PATCH 1/4] Bug #27213: avoid failed assertions for non-executable directories.
To: [email protected]

Addresses Savannah bug #27213 - https://savannah.gnu.org/bugs/?27213

This used to fail in recent releases:
1. mkdir -p foo/bar
2. chmod a-x foo
3. find foo
4. find foo -ls

Now it will print error messages for the denied permission on foo, but will
not abort completely, and in 3. will even print the name foo/bar, while it
won't do so in 4., as the -ls predicate requires stat information. For now,
4. will print two identical error message. This should get fixed some day.
---
 build-aux/.cvsignore |    1 +
 build-aux/.gitignore |    1 +
 find/ftsfind.c       |   19 ++++++++++++++++++-
 find/util.c          |   11 +++++++----
 4 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/build-aux/.cvsignore b/build-aux/.cvsignore
index f5e817b..35d182e 100644
--- a/build-aux/.cvsignore
+++ b/build-aux/.cvsignore
@@ -18,3 +18,4 @@ c++defs.h
 useless-if-before-free
 vc-list-files
 update-copyright
+unused-parameter.h
diff --git a/build-aux/.gitignore b/build-aux/.gitignore
index 7ced4ed..be5fe6e 100644
--- a/build-aux/.gitignore
+++ b/build-aux/.gitignore
@@ -17,3 +17,4 @@ compile
 /useless-if-before-free
 /vc-list-files
 /update-copyright
+/unused-parameter.h
diff --git a/find/ftsfind.c b/find/ftsfind.c
index d909579..4b9dca2 100644
--- a/find/ftsfind.c
+++ b/find/ftsfind.c
@@ -459,6 +459,23 @@ consider_visiting (FTS *p, FTSENT *ent)
 	      error_severity (1);
 	      return;
 	    }
+	  else
+	    {
+	      error(0, ent->fts_errno, "%s",
+		    safely_quote_err_filename(0, ent->fts_path));
+	      error_severity(1);
+	      /* Continue despite the error, as file name without stat info
+	       * might be better than not even processing the file name. This
+	       * can lead to repeated error messages later on, though, if a
+	       * predicate requires stat information.
+	       *
+	       * Not printing an error message here would be even more wrong,
+	       * though, as this could cause the contents of a directory to be
+	       * silently ignored, as the directory wouldn't be identified as
+	       * such.
+	       */
+	    }
+	  
 	}
     }
 
@@ -467,7 +484,7 @@ consider_visiting (FTS *p, FTSENT *ent)
       || ent->fts_info == FTS_NS /* e.g. symlink loop */)
     {
       assert (!state.have_stat);
-      assert (ent->fts_info == FTS_NSOK || state.type != 0);
+      assert (ent->fts_info == FTS_NSOK || state.type == 0);
       mode = state.type;
     }
   else
diff --git a/find/util.c b/find/util.c
index e19df6f..9693e57 100644
--- a/find/util.c
+++ b/find/util.c
@@ -216,6 +216,9 @@ get_statinfo (const char *pathname, const char *name, struct stat *p)
 	{
 	  if (!options.ignore_readdir_race || (errno != ENOENT) )
 	    {
+              /* FIXME: this error message might repeat the one from
+               * the FTS_NS case in consider_visiting. How to avoid this?
+               */
 	      error (0, errno, "%s",
 		     safely_quote_err_filename (0, pathname));
 	      state.exit_status = 1;
@@ -272,6 +275,10 @@ get_info (const char *pathname,
       int result = get_statinfo (pathname, state.rel_pathname, p);
       if (result != 0)
 	{
+	  return -1;		/* failure. */
+	}
+      else
+	{
 	  /* Verify some postconditions.  We can't check st_mode for
 	     non-zero-ness because of Savannah bug #16378 (which is
 	     that broken NFS servers can return st_mode==0). */
@@ -283,10 +290,6 @@ get_info (const char *pathname,
 	    {
 	      assert (p->st_ino);
 	    }
-	  return -1;		/* failure. */
-	}
-      else
-	{
 	  return 0;		/* success. */
 	}
     }
-- 
1.7.0

From ddbfa03acd5c9766f2fb99290856c8bb5de93649 Mon Sep 17 00:00:00 2001
From: James Youngman <[email protected]>
Date: Sun, 4 Apr 2010 17:14:31 +0100
Subject: [PATCH 2/4] Don't issue an error message twice for the same target file.
To: [email protected]

* find/defs.h (struct state): New member,
already_issued_stat_error_msg, used to de-duplicate error
messages.  If it is true, we already issued an error message for
the current target file.
Declare fatal_target_file_error, fatal_nontarget_file_error,
nonfatal_target_file_error, nonfatal_nontarget_file_error.
Between them, they replace fatal_file_error and
nonfatal_file_error.  The *target_file_error versions are
de-duplicated and the nontarget_file_error_versions are not.
* find/util.c (nonfatal_nontarget_file_error): Implement.
(fatal_nontarget_file_error): Implement.
(nonfatal_target_file_error): Implement.
(fatal_target_file_error): Implement.
(fatal_file_error): Remove.
(nonfatal_file_error): Remove.
(error_severity): Define error_severity (moved from ftsfind.c)
(get_statinfo): Call nonfatal_target_file_error in order to avoid
a duplicate message.  ALso call error_severity after a different
call to error to preserve the constraint that we exit with a
nonzero status if we issue a diagnostic.
(cleanup): Call nonfatal_nontarget_file_error instead of
nonfatal_file_error.
* find/ftsfind.c (error_severity): Move to util.c.
* find/tree.c (build_expression_tree): Initialise
state.already_issued_stat_error_msg.
* find/find.c (main): Initialise state.already_issued_stat_error_msg.
(wd_sanity_check): Call fatal_target_file_error instead of
fatal_file_error.
(chdir_back): Call fatal_nontarget_file_error instead of
fatal_file_error.
(process_path): Initialise state.already_issued_stat_error_msg.
* find/ftsfind.c (consider_visiting): Call
nonfatal_target_file_error instead of calling error directly.
(find): Call error_severity to ensure exit status is nonzero after
a call to error.
(main): Initialise state.already_issued_stat_error_msg.
* find/parser.c (collect_arg_stat_info): Call
fatal_target_file_error instead of fatal_file_error.
(parse_newerXY): Likewise.
(parse_samefile): Likewise.
(parse_samefile): Likewise.
(open_output_file): Call fatal_nontarget_file_error instead of
fatal_file_error.
* find/pred.c (checked_fprintf): Likewise.
(checked_print_quoted): Likewise.
(checked_fwrite): Likewise.
(checked_fflush): Likewise.
* find/sharefile.c (entry_free): Likewise.

Signed-off-by: James Youngman <[email protected]>
---
 ChangeLog        |   60 +++++++++++++++++++++++++++++++++++++++++
 find/defs.h      |   12 +++++++-
 find/find.c      |    8 +++--
 find/ftsfind.c   |   42 +++++++---------------------
 find/parser.c    |   10 +++---
 find/pred.c      |    8 +++---
 find/sharefile.c |    2 +-
 find/tree.c      |    1 +
 find/util.c      |   78 ++++++++++++++++++++++++++++++++++++++++++------------
 9 files changed, 158 insertions(+), 63 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index d044174..29bde02 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,65 @@
 2010-04-05  James Youngman  <[email protected]>
 
+	Don't issue an error message twice for the same target file.
+	* find/defs.h (struct state): New member,
+	already_issued_stat_error_msg, used to de-duplicate error
+	messages.  If it is true, we already issued an error message for
+	the current target file.
+	Declare fatal_target_file_error, fatal_nontarget_file_error,
+	nonfatal_target_file_error, nonfatal_nontarget_file_error.
+	Between them, they replace fatal_file_error and
+	nonfatal_file_error.  The *target_file_error versions are
+	de-duplicated and the nontarget_file_error_versions are not.
+	* find/util.c (nonfatal_nontarget_file_error): Implement.
+	(fatal_nontarget_file_error): Implement.
+	(nonfatal_target_file_error): Implement.
+	(fatal_target_file_error): Implement.
+	(fatal_file_error): Remove.
+	(nonfatal_file_error): Remove.
+	(error_severity): Define error_severity (moved from ftsfind.c)
+	(get_statinfo): Call nonfatal_target_file_error in order to avoid
+	a duplicate message.  ALso call error_severity after a different
+	call to error to preserve the constraint that we exit with a
+	nonzero status if we issue a diagnostic.
+	(cleanup): Call nonfatal_nontarget_file_error instead of
+	nonfatal_file_error.
+	* find/ftsfind.c (error_severity): Move to util.c.
+	* find/tree.c (build_expression_tree): Initialise
+	state.already_issued_stat_error_msg.
+	* find/find.c (main): Initialise state.already_issued_stat_error_msg.
+	(wd_sanity_check): Call fatal_target_file_error instead of
+	fatal_file_error.
+	(chdir_back): Call fatal_nontarget_file_error instead of
+	fatal_file_error.
+	(process_path): Initialise state.already_issued_stat_error_msg.
+	* find/ftsfind.c (consider_visiting): Call
+	nonfatal_target_file_error instead of calling error directly.
+	(find): Call error_severity to ensure exit status is nonzero after
+	a call to error.
+	(main): Initialise state.already_issued_stat_error_msg.
+	* find/parser.c (collect_arg_stat_info): Call
+	fatal_target_file_error instead of fatal_file_error.
+	(parse_newerXY): Likewise.
+	(parse_samefile): Likewise.
+	(parse_samefile): Likewise.
+	(open_output_file): Call fatal_nontarget_file_error instead of
+	fatal_file_error.
+	* find/pred.c (checked_fprintf): Likewise.
+	(checked_print_quoted): Likewise.
+	(checked_fwrite): Likewise.
+	(checked_fflush): Likewise.
+	* find/sharefile.c (entry_free): Likewise.
+
+	Fix Savannah bug #29435: fd_is_cloexec does not work on Fedora
+	buildhosts.
+	Fix open_cloexec on hosts which ignore O_CLOEXEC (i.e. old kernels).
+	* lib/fdleak.c (o_cloexec_works): New function, detects whether
+	the open flag O_CLOEXEC has any effect.
+	(open_cloexec): Call o_cloexec_works, just once, to find out
+	whether O_CLOEXEC is effective.  If not, set the close-on-exec
+	flag on fds by calling set_cloexec_flag.
+	* NEWS: Mention this bugfix.
+
 	Use bool instead of the previous typedef boolean.
 	* find/defs.h: Don't create typedef "boolean"; use the standard
 	type bool.  Update other declarations accordingly.
diff --git a/find/defs.h b/find/defs.h
index f0750da..e7892f3 100644
--- a/find/defs.h
+++ b/find/defs.h
@@ -502,11 +502,16 @@ extern bool check_nofollow(void);
 void complete_pending_execs(struct predicate *p);
 void complete_pending_execdirs(int dir_fd); /* Passing dir_fd is an unpleasant CodeSmell. */
 const char *safely_quote_err_filename (int n, char const *arg);
-void fatal_file_error(const char *name) ATTRIBUTE_NORETURN;
-void nonfatal_file_error(const char *name);
+
+void fatal_target_file_error (int errno_value, const char *name) ATTRIBUTE_NORETURN;
+void fatal_nontarget_file_error (int errno_value, const char *name) ATTRIBUTE_NORETURN;
+void nonfatal_target_file_error (int erron_value, const char *name);
+void nonfatal_nontarget_file_error (int erron_value, const char *name);
+
 
 int process_leading_options PARAMS((int argc, char *argv[]));
 void set_option_defaults PARAMS((struct options *p));
+void error_severity PARAMS((int level));
 
 #if 0
 #define apply_predicate(pathname, stat_buf_ptr, node)	\
@@ -664,6 +669,9 @@ struct state
 
   /* Shared files, opened via the interface in sharefile.h. */
   sharefile_handle shared_files;
+
+  /* Avoid multiple error messages for the same file. */
+  boolean already_issued_stat_error_msg;
 };
 
 /* finddata.c */
diff --git a/find/find.c b/find/find.c
index f5be91d..04e2968 100644
--- a/find/find.c
+++ b/find/find.c
@@ -136,6 +136,7 @@ main (int argc, char **argv)
       remember_non_cloexec_fds ();
     }
 
+  state.already_issued_stat_error_msg = false;
   state.shared_files = sharefile_init ("w");
   if (NULL == state.shared_files)
     {
@@ -473,7 +474,7 @@ wd_sanity_check (const char *thing_to_stat,
 
   set_stat_placeholders (newinfo);
   if ((*options.xstat) (current_dir, newinfo) != 0)
-    fatal_file_error (thing_to_stat);
+    fatal_target_file_error (errno, thing_to_stat);
 
   if (old_dev != newinfo->st_dev)
     {
@@ -944,7 +945,7 @@ chdir_back (void)
 #endif
 
       if (chdir (starting_dir) != 0)
-	fatal_file_error (starting_dir);
+	fatal_nontarget_file_error (errno, starting_dir);
 
       wd_sanity_check (starting_dir,
 		       program_name,
@@ -963,7 +964,7 @@ chdir_back (void)
 
       if (fchdir (starting_desc) != 0)
 	{
-	  fatal_file_error (starting_dir);
+	  fatal_nontarget_file_error (errno, starting_dir);
 	}
     }
 }
@@ -1184,6 +1185,7 @@ process_path (char *pathname, char *name, bool leaf, char *parent,
   state.type = 0;
   state.have_stat = false;
   state.have_type = false;
+  state.already_issued_stat_error_msg = false;
 
   if (!digest_mode (&mode, pathname, name, &stat_buf, leaf))
     return 0;
diff --git a/find/ftsfind.c b/find/ftsfind.c
index 4b9dca2..aa086a4 100644
--- a/find/ftsfind.c
+++ b/find/ftsfind.c
@@ -173,18 +173,6 @@ inside_dir (int dir_fd)
 static void init_mounted_dev_list (void);
 #endif
 
-/* 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
- * reduce it to 1.
- */
-static void
-error_severity (int level)
-{
-  if (state.exit_status < level)
-    state.exit_status = level;
-}
-
 
 #define STRINGIFY(X) #X
 #define HANDLECASE(N) case N: return #N;
@@ -371,9 +359,6 @@ show_outstanding_execdirs (FILE *fp)
       /* No debug output is wanted. */
     }
 }
-
-
-
 
 static void
 consider_visiting (FTS *p, FTSENT *ent)
@@ -410,15 +395,13 @@ consider_visiting (FTS *p, FTSENT *ent)
   if (ent->fts_info == FTS_ERR
       || ent->fts_info == FTS_DNR)
     {
-      error (0, ent->fts_errno, "%s",
-	     safely_quote_err_filename (0, ent->fts_path));
-      error_severity (1);
+      nonfatal_target_file_error (ent->fts_errno, ent->fts_path);
       return;
     }
   else if (ent->fts_info == FTS_DC)
     {
       issue_loop_warning (ent);
-      error_severity (1);
+      error_severity (EXIT_FAILURE);
       return;
     }
   else if (ent->fts_info == FTS_SLNONE)
@@ -432,8 +415,7 @@ consider_visiting (FTS *p, FTSENT *ent)
        */
       if (symlink_loop (ent->fts_accpath))
 	{
-	  error (0, ELOOP, "%s", safely_quote_err_filename (0, ent->fts_path));
-	  error_severity (1);
+	  nonfatal_target_file_error (ELOOP, ent->fts_path);
 	  return;
 	}
     }
@@ -442,9 +424,7 @@ consider_visiting (FTS *p, FTSENT *ent)
       if (ent->fts_level == 0)
 	{
 	  /* e.g., nonexistent starting point */
-	  error (0, ent->fts_errno, "%s",
-		 safely_quote_err_filename (0, ent->fts_path));
-	  error_severity (1);	/* remember problem */
+	  nonfatal_target_file_error (ent->fts_errno, ent->fts_path);
 	  return;
 	}
       else
@@ -454,16 +434,12 @@ consider_visiting (FTS *p, FTSENT *ent)
 	   */
 	  if (symlink_loop (ent->fts_accpath))
 	    {
-	      error (0, ELOOP, "%s",
-		     safely_quote_err_filename (0, ent->fts_path));
-	      error_severity (1);
+	      nonfatal_target_file_error (ELOOP, ent->fts_path);
 	      return;
 	    }
 	  else
 	    {
-	      error(0, ent->fts_errno, "%s",
-		    safely_quote_err_filename(0, ent->fts_path));
-	      error_severity(1);
+	      nonfatal_target_file_error (ent->fts_errno, ent->fts_path);
 	      /* Continue despite the error, as file name without stat info
 	       * might be better than not even processing the file name. This
 	       * can lead to repeated error messages later on, though, if a
@@ -475,7 +451,7 @@ consider_visiting (FTS *p, FTSENT *ent)
 	       * such.
 	       */
 	    }
-	  
+
 	}
     }
 
@@ -632,11 +608,13 @@ find (char *arg)
     {
       error (0, errno, _("cannot search %s"),
 	     safely_quote_err_filename (0, arg));
+      error_severity (EXIT_FAILURE);
     }
   else
     {
       while ( (ent=fts_read (p)) != NULL )
 	{
+	  state.already_issued_stat_error_msg = false;
 	  state.have_stat = false;
 	  state.have_type = !!ent->fts_statp->st_mode;
 	  state.type = state.have_type ? ent->fts_statp->st_mode : 0;
@@ -651,6 +629,7 @@ find (char *arg)
 	  error (0, errno,
 		 _("failed to restore working directory after searching %s"),
 		 arg);
+	  error_severity (EXIT_FAILURE);
 	  return false;
 	}
       p = NULL;
@@ -700,6 +679,7 @@ main (int argc, char **argv)
   else
     set_program_name ("find");
 
+  state.already_issued_stat_error_msg = false;
   state.exit_status = 0;
   state.execdirs_outstanding = false;
   state.cwd_dir_fd = AT_FDCWD;
diff --git a/find/parser.c b/find/parser.c
index bb09ed0..44dff55 100644
--- a/find/parser.c
+++ b/find/parser.c
@@ -742,7 +742,7 @@ collect_arg_stat_info (char **argv, int *arg_ptr, struct stat *p,
 	}
       else
 	{
-	  fatal_file_error (filename);
+	  fatal_target_file_error (errno, filename);
 	}
     }
   else
@@ -1683,7 +1683,7 @@ parse_newerXY (const struct parser_table* entry, char **argv, int *arg_ptr)
 	      /* Stat the named file. */
 	      set_stat_placeholders (&stat_newer);
 	      if ((*options.xstat) (argv[*arg_ptr], &stat_newer))
-		fatal_file_error (argv[*arg_ptr]);
+		fatal_target_file_error (errno, argv[*arg_ptr]);
 
 	      if (!get_stat_Ytime (&stat_newer, y, &our_pred->args.reftime.ts))
 		{
@@ -2423,7 +2423,7 @@ parse_samefile (const struct parser_table* entry, char **argv, int *arg_ptr)
 	   */
 	  if (0 != fstat (fd, &fst))
 	    {
-	      fatal_file_error (argv[*arg_ptr]);
+	      fatal_target_file_error (errno, argv[*arg_ptr]);
 	    }
 	  else
 	    {
@@ -2433,7 +2433,7 @@ parse_samefile (const struct parser_table* entry, char **argv, int *arg_ptr)
 	       * destination of the link, not the link itself.
 	       */
 	      if ((*options.xstat) (argv[*arg_ptr], &st))
-		fatal_file_error (argv[*arg_ptr]);
+		fatal_target_file_error (errno, argv[*arg_ptr]);
 
 	      if ((options.symlink_handling == SYMLINK_NEVER_DEREF)
 		  && (!options.open_nofollow_available))
@@ -3787,7 +3787,7 @@ open_output_file (const char *path, struct format_val *p)
 
       if (p->stream == NULL)
 	{
-	  fatal_file_error (path);
+	  fatal_nontarget_file_error (errno, path);
 	}
     }
 
diff --git a/find/pred.c b/find/pred.c
index 6850372..918db2c 100644
--- a/find/pred.c
+++ b/find/pred.c
@@ -696,7 +696,7 @@ checked_fprintf (struct format_val *dest, const char *fmt, ...)
   va_start (ap, fmt);
   rv = vfprintf (dest->stream, fmt, ap);
   if (rv < 0)
-    nonfatal_file_error (dest->filename);
+    nonfatal_nontarget_file_error (errno, dest->filename);
 }
 
 
@@ -707,7 +707,7 @@ checked_print_quoted (struct format_val *dest,
   int rv = print_quoted (dest->stream, dest->quote_opts, dest->dest_is_tty,
 			 format, s);
   if (rv < 0)
-    nonfatal_file_error (dest->filename);
+    nonfatal_nontarget_file_error (errno, dest->filename);
 }
 
 
@@ -716,7 +716,7 @@ checked_fwrite (void *p, size_t siz, size_t nmemb, struct format_val *dest)
 {
   int items_written = fwrite (p, siz, nmemb, dest->stream);
   if (items_written < nmemb)
-    nonfatal_file_error (dest->filename);
+    nonfatal_nontarget_file_error (errno, dest->filename);
 }
 
 static void
@@ -724,7 +724,7 @@ checked_fflush (struct format_val *dest)
 {
   if (0 != fflush (dest->stream))
     {
-      nonfatal_file_error (dest->filename);
+      nonfatal_nontarget_file_error (errno, dest->filename);
     }
 }
 
diff --git a/find/sharefile.c b/find/sharefile.c
index 8e0f4cc..1442d7b 100644
--- a/find/sharefile.c
+++ b/find/sharefile.c
@@ -76,7 +76,7 @@ entry_free (void *pv)
   if (p->fp)
     {
       if (0 != fclose (p->fp))
-	fatal_file_error (p->name);
+	fatal_nontarget_file_error (errno, p->name);
     }
   free (p->name);
   free (p);
diff --git a/find/tree.c b/find/tree.c
index 15e3129..a9cf3d7 100644
--- a/find/tree.c
+++ b/find/tree.c
@@ -1284,6 +1284,7 @@ build_expression_tree (int argc, char *argv[], int end_of_leading_options)
   /* Build the input order list. */
   while (i < argc )
     {
+      state.already_issued_stat_error_msg = false;
       if (!looks_like_expression (argv[i], false))
 	{
 	  error (0, 0, _("paths must precede expression: %s"), argv[i]);
diff --git a/find/util.c b/find/util.c
index 9693e57..62d80c8 100644
--- a/find/util.c
+++ b/find/util.c
@@ -210,18 +210,14 @@ get_statinfo (const char *pathname, const char *name, struct stat *p)
 	      /* Savannah bug #16378. */
 	      error (0, 0, _("WARNING: file %s appears to have mode 0000"),
 		     quotearg_n_style (0, options.err_quoting_style, name));
+	      error_severity (1);
 	    }
 	}
       else
 	{
 	  if (!options.ignore_readdir_race || (errno != ENOENT) )
 	    {
-              /* FIXME: this error message might repeat the one from
-               * the FTS_NS case in consider_visiting. How to avoid this?
-               */
-	      error (0, errno, "%s",
-		     safely_quote_err_filename (0, pathname));
-	      state.exit_status = 1;
+	      nonfatal_target_file_error (errno, pathname);
 	    }
 	  return -1;
 	}
@@ -489,7 +485,7 @@ cleanup (void)
     }
 
   if (fflush (stdout) == EOF)
-    nonfatal_file_error ("standard output");
+    nonfatal_nontarget_file_error (errno, "standard output");
 }
 
 /* Savannah bug #16378 manifests as an assertion failure in pred_type()
@@ -1077,35 +1073,83 @@ safely_quote_err_filename (int n, char const *arg)
   return quotearg_n_style (n, options.err_quoting_style, arg);
 }
 
+/* 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
+ * reduce it to 1.
+ */
+void
+error_severity (int level)
+{
+  if (state.exit_status < level)
+    state.exit_status = level;
+}
+
 /* report_file_err
  */
 static void
-report_file_err(int exitval, int errno_value, const char *name)
+report_file_err(int exitval, int errno_value,
+		boolean is_target_file, const char *name)
 {
   /* It is important that the errno value is passed in as a function
    * argument before we call safely_quote_err_filename(), because otherwise
    * we might find that safely_quote_err_filename() changes errno.
    */
-  if (state.exit_status < 1)
-    state.exit_status = 1;
-
-  error (exitval, errno_value, "%s", safely_quote_err_filename (0, name));
+  if (!is_target_file || !state.already_issued_stat_error_msg)
+    {
+      error (exitval, errno_value, "%s", safely_quote_err_filename (0, name));
+      error_severity (1);
+    }
+  if (is_target_file)
+    {
+      state.already_issued_stat_error_msg = true;
+    }
 }
 
-/* fatal_file_error
+/* nonfatal_target_file_error
  *
  */
 void
-fatal_file_error(const char *name)
+nonfatal_target_file_error (int errno_value, const char *name)
 {
-  report_file_err (1, errno, name);
+  report_file_err (0, errno_value, true, name);
+}
+
+/* fatal_target_file_error
+ *
+ * Report an error on a target file (i.e. a file we are searching).
+ * Such errors are only reported once per searched file.
+ *
+ */
+void
+fatal_target_file_error(int errno_value, const char *name)
+{
+  report_file_err (1, errno_value, true, name);
   /*NOTREACHED*/
   abort ();
 }
 
+/* nonfatal_nontarget_file_error
+ *
+ */
 void
-nonfatal_file_error (const char *name)
+nonfatal_nontarget_file_error (int errno_value, const char *name)
 {
-  report_file_err (0, errno, name);
+  report_file_err (0, errno_value, false, name);
 }
 
+/* fatal_nontarget_file_error
+ *
+ */
+void
+fatal_nontarget_file_error(int errno_value, const char *name)
+{
+  /* We're going to exit fatally, so make sure we always isssue the error
+   * message, even if it will be duplicate.   Motivation: otherwise it may
+   * not be clear what went wrong.
+   */
+  state.already_issued_stat_error_msg = false;
+  report_file_err (1, errno_value, false, name);
+  /*NOTREACHED*/
+  abort ();
+}
-- 
1.7.0

From d5d86329fb9ae6939a2fc74cce6f3b0feef61b92 Mon Sep 17 00:00:00 2001
From: James Youngman <[email protected]>
Date: Sun, 4 Apr 2010 17:24:39 +0100
Subject: [PATCH 3/4] Add ChangeLog entry for Matin's change
To: [email protected]

---
 ChangeLog |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 29bde02..a9d1859 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -23,7 +23,6 @@
 	nonzero status if we issue a diagnostic.
 	(cleanup): Call nonfatal_nontarget_file_error instead of
 	nonfatal_file_error.
-	* find/ftsfind.c (error_severity): Move to util.c.
 	* find/tree.c (build_expression_tree): Initialise
 	state.already_issued_stat_error_msg.
 	* find/find.c (main): Initialise state.already_issued_stat_error_msg.
@@ -37,6 +36,7 @@
 	(find): Call error_severity to ensure exit status is nonzero after
 	a call to error.
 	(main): Initialise state.already_issued_stat_error_msg.
+	(error_severity): Move to util.c.
 	* find/parser.c (collect_arg_stat_info): Call
 	fatal_target_file_error instead of fatal_file_error.
 	(parse_newerXY): Likewise.
@@ -50,6 +50,20 @@
 	(checked_fflush): Likewise.
 	* find/sharefile.c (entry_free): Likewise.
 
+2010-04-04  Martin von Gagern  <[email protected]>
+
+	Fix Savannah bug #27213: avoid failed assertions for
+	non-executable directories.
+	(consider_visiting): Continue (after issuing an error message)
+	even if ent->fts_info == FTS_NS.
+	* find/util.c (get_statinfo): If we cannot stat the file, issue a
+	diagnostic, but continue anyway.
+	* find/ftsfind.c (consider_visiting): Don't assert-fail if
+	ent->fts_info == FTS_NSOK.  Don't assert-fail if state.type is
+	nonzero.
+
+2010-04-04  James Youngman  <[email protected]>
+
 	Fix Savannah bug #29435: fd_is_cloexec does not work on Fedora
 	buildhosts.
 	Fix open_cloexec on hosts which ignore O_CLOEXEC (i.e. old kernels).
-- 
1.7.0

From 2d2036425843e47d9fdcd758d339f3390d8e7c59 Mon Sep 17 00:00:00 2001
From: James Youngman <[email protected]>
Date: Sun, 4 Apr 2010 17:29:21 +0100
Subject: [PATCH 4/4] Add NEWS file entry too.
To: [email protected]

---
 NEWS |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/NEWS b/NEWS
index d798cf2..7700752 100644
--- a/NEWS
+++ b/NEWS
@@ -6,6 +6,7 @@ GNU findutils NEWS - User visible changes.      -*- outline -*- (allout)
 
 #29435: fd_is_cloexec does not work on Fedora buildhosts
 
+#27213: avoid failed assertions for non-executable directories.
 
 * Major changes in release 4.5.7, 2010-04-03
 
-- 
1.7.0

_______________________________________________
Findutils-patches mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/findutils-patches

Reply via email to