On Fri, May 7, 2010 at 9:21 AM, Kamil Dudka <[email protected]> wrote:
> On Friday 07 of May 2010 10:08:08 James Youngman wrote:
>> Thanks for spotting this.   I can see the bug simply by inspecting
>> your fix.   But, is there any kind of test case which actually
>> demonstrates the problem?   If we add a relevant test we can protect
>> ourselves against reintroducing this kind of bug, too.
>
> As I said, I hit the bug in 4.4.2.  The code that the patch addresses works
> in 4.5.9 without the patch, but not in 4.4.2.  The failing test-case was
> execdir-multiple.exp.

I see.  Thanks.

I have applied and pushed your patch as the two attached git patches
(if you could send your changes in "git format-patch" format in
future, ideally with a ChangeLog entry too, that would streamline
things for me).

Thank you for this contribution to GNU findutils.

> It simply broke the traversal as soon as the first
> execdir had been executed.  FWIW the full backport for upstream bugs #19593
> and #27563 is attached to the corresponding Fedora bug:
>
> https://bugzilla.redhat.com/493143

Thanks for the pointer.   The semantics that Gen Zhang was relying on
(processing all entries in a directory at once) were never guaranteed
anyway.

If you'd like to supply a backported "format-patch" patch against the
4.4.x tree, I can apply that too.

Thanks,
James.
From 443487e98929bf8e5e8bc7816f605a6f0924dfe7 Mon Sep 17 00:00:00 2001
From: Kamil Dudka <[email protected]>
Date: Fri, 7 May 2010 09:29:17 +0100
Subject: [PATCH 1/2] Correctly initialise variables in run_in_dir.
To: [email protected]

* lib/dircallback.c (run_in_dir): Make sure that if the callback
doesn't get run, the return value is nonzero.  Make sure that if
the directory save/restore fails, we don't overwrite errno with a
random value (and hence report some unrelated and nonexistent
error, instead of the real problem).  Restore the previous current
directory.

Signed-off-by: James Youngman <[email protected]>
---
 ChangeLog         |    9 +++++++++
 lib/dircallback.c |    7 ++++++-
 2 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 02bfaa0..8e6de22 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2010-05-07  Kamil Dudka  <[email protected]>
+
+	* lib/dircallback.c (run_in_dir): Make sure that if the callback
+	doesn't get run, the return value is nonzero.  Make sure that if
+	the directory save/restore fails, we don't overwrite errno with a
+	random value (and hence report some unrelated and nonexistent
+	error, instead of the real problem).  Restore the previous current
+	directory.
+
 2010-05-04  James Youngman  <[email protected]>
 
 	Bugfix: make sure make distdir works in VPATH directly after configure
diff --git a/lib/dircallback.c b/lib/dircallback.c
index 8497bee..c1e4088 100644
--- a/lib/dircallback.c
+++ b/lib/dircallback.c
@@ -37,7 +37,8 @@ int
 run_in_dir (const struct saved_cwd *there,
 	    int (*callback)(void*), void *usercontext)
 {
-  int err, saved_errno;
+  int err = -1;
+  int saved_errno = 0;
   struct saved_cwd here;
   if (0 == save_cwd (&here))
     {
@@ -50,6 +51,10 @@ run_in_dir (const struct saved_cwd *there,
 	{
 	  openat_restore_fail (errno);
 	}
+
+      if (restore_cwd (&here) != 0)
+	openat_restore_fail (errno);
+
       free_cwd (&here);
     }
   else
-- 
1.7.0

From d15d752dbc318c9ca122db73a4aa91cfbc3715cc Mon Sep 17 00:00:00 2001
From: James Youngman <[email protected]>
Date: Fri, 7 May 2010 09:31:10 +0100
Subject: [PATCH 2/2] Add Kamil Dudka to the AUTHORS file.
To: [email protected]

* AUTHORS: Add Kamil Dudka.
---
 AUTHORS   |    1 +
 ChangeLog |    4 ++++
 2 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/AUTHORS b/AUTHORS
index de84fd1..5038582 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -29,6 +29,7 @@ Dmitry V. Levin
 Bas van Gompel
 Eric Blake <[email protected]>
 Andreas Metzler
+Kamil Dudka  <[email protected]>
 
 The current maintainer of the findutils package is James Youngman.
 Questions about findutils should be addressed to the mailing list,
diff --git a/ChangeLog b/ChangeLog
index 8e6de22..70d305b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2010-05-07  James Youngman  <[email protected]>
+
+	* AUTHORS: Add Kamil Dudka.
+
 2010-05-07  Kamil Dudka  <[email protected]>
 
 	* lib/dircallback.c (run_in_dir): Make sure that if the callback
-- 
1.7.0

Reply via email to