bugfindutils - Bugs: bug #18466, -execdir {} + broken in new find

 
 

bug #18466: -execdir {} + broken in new find

Submitted by:  Eric Blake <ericb>
Submitted on:  Wed 06 Dec 2006 04:29:05 AM UTC  
 
Category:  find Severity:  4 - Important
Item Group:  Wrong result Status:  Fixed
Privacy:  Public Assigned to:  James Youngman <jay>
Originator Name:  Eric Blake Open/Closed:  Closed
Release:  4.3.2 Fixed Release:  4.3.3

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

Sat 14 Apr 2007 10:06:44 PM UTC, comment #6: 

Actually I'm going to makr this as fixed, since essentially it is.  The resulting performance degradation is tracked as bug #19593.

James Youngman <jay>
Project AdministratorIn charge of this item.
Sat 14 Apr 2007 09:43:56 PM UTC, comment #5: 

There is a related bug which bits on the way down rather than up.  Ugh.  I have worked around the problem in CVS by limiting the build-up of -execdir ...+ arguments.  That's quite unpleasant.   I hope for a better fix soon and so am leaving this bug open.

James Youngman <jay>
Project AdministratorIn charge of this item.
Sat 14 Apr 2007 02:50:52 PM UTC, comment #4: 

I attach a patch which switches ftsfind.c to using FTS_CWDFD.  Unfortunately, this involved many changes in all sorts of places, since many filesystem operations (such as readlink() and access()) have no *at() equivalent.

The patch should apply against the current findutils CVS code (against HEAD, which is currently the findutils-4.3.x development code).

I haven't decided yet whether or not to go this way.

(file #12480)

James Youngman <jay>
Project AdministratorIn charge of this item.
Mon 22 Jan 2007 10:59:29 AM UTC, comment #3: 

I have enhanced the test suite to test for this bug (but not yet fixed the bug itself).

James Youngman <jay>
Project AdministratorIn charge of this item.
Thu 07 Dec 2006 12:49:00 AM UTC, comment #2: 

The last patch has a flaw; it only worked for 'find -P /absolute'.  I'm attaching an improved version, but with the same caveat that moving to FTS_CWDFD would be a better solution than my hack.

(file #11466)

Eric Blake <ericb>
Project Administrator
Wed 06 Dec 2006 05:04:31 AM UTC, comment #1: 

This patch is a quick hack; it won't work on systems like mingw that lack fchdir (although this is not much of a loss, since gnulib's fts currently requires fchdir, and will be resolved once someone contributes a gnulib fchdir module), or where . cannot be returned to (also not much of a loss, since without FTS_CWDFD, you are already performing chdirs, so it is only extreme race cases that can cause problems, and this is not adding to the vulnerability).  Since it is a hack, I did not bother checking for failure; all the more reason to not accept this patch as-is.  But with this hack applied, the testcase mentioned in the original report uses the correct directory.

IMO, a better solution would be to use gnulib's FTS_CWDFD so that the traversal never changes working directories, then do fchdir inside launch(), after the fork but before the exec.  But converting ftsfind to use FTS_CWDFD seemed like a more time-consuming project, not to mention that all callers of launch would need a change to pass the target working directory fd.

2006-12-05  Eric Blake  <ebb9@byu.net>

Hack - use at your own risk:

  • find/ftsfind.c (consider_visiting): Save and restore

directories, so that -execdir runs in the correct location.

(file #11460)

Eric Blake <ericb>
Project Administrator
Wed 06 Dec 2006 04:29:05 AM UTC, original submission:  

The change from oldfind to the fts algorithm broke -execdir {} +:

http://cygwin.com/ml/cygwin/2006-12/msg00203.html

The command is being run in the parent directory rather than the directory that provided the current set of {} argument expansions.

I'm not sure of the fix off hand, but it seems like leaving a directory is the trigger to run the command on the collected arguments, but it triggers after the working dir has already been changed.  You either need to trigger sooner, or temporarily revert the working directory back to where the arguments were collected.

Eric Blake <ericb>
Project Administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #12481:  dircallback.patch added by jay (4KiB - text/x-patch)
file #12480:  execdir-pwd.patch added by jay (38KiB - text/x-patch)
file #11466:  findutils.patch3 added by ericb (831B - application/octet-stream - improved version of hack)
file #11460:  findutils.patch2 added by ericb (620B - text/plain - hack patch for -execdir {} +)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jay (Posted a comment)
  • -email is unavailable- added by ericb (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    Follow 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2007-04-15 jay Open/ClosedOpen => Closed
        Fixed ReleaseNone => 4.3.3
    2007-04-14 jay StatusIn Progress => Fixed
    2007-04-14 jay StatusNone => In Progress
    2007-04-14 jay Attached File- => Added dircallback.patch, #12481
    2007-04-14 jay Attached File- => Added execdir-pwd.patch, #12480
        Assigned toNone => jay
    2006-12-07 ericb Attached File- => Added findutils.patch3, #11466
    2006-12-06 ericb Attached File- => Added findutils.patch2, #11460

    Back to the top


    Powered by Savane 3.5