Fwd: This week's NEWS

[James Youngman]

---------- Forwarded message ----------
From: James Youngman <address@hidden>
Date: Jun 3, 2007 9:24 AM
Subject: Re: This week's NEWS
To: address@hidden


On 6/2/07, address@hidden <address@hidden> wrote:
> Dear James,
>
> I just committed this week's changes.

Thanks.   I am copying the list on the email, hope you don't mind.

[edit: this time, actually sending it to the list]


> ext2 attribute support is working fine, now (relying on the library, as
> requested).
>
> I noticed two things:
>
>     1. in the few places where base_name() is used, the memory is not
>        freed.  Is this intentional?  If not, I can fix it.

It's a hangover from the days when we used to use basename().  The
basename() function does not allocate memory.    The fact that the
buffer returned by base_name() is not freed is not intentional.   I'm
sure the same problem affects both 4.3.x and 4.2.x.   If you fix it,
could you mail it to me as a separate patch?

>     2. I didn't know that find changes the cwd before calling predicate
>        functions.  This behaviour, or rather my ignorance concerning it,
>        led to pred_ext2attr() giving apparently mysterious "file not
>        found" errors.  This was the more irritating as pred_xattr() did
>        not show this behaviour

Do they need to be two separate predicates?

>     -- because you seem to have added a clause
>        to ignore ENOENTs [1].  Why did you do that?  Can I remove it?
>        It doesn't seem to make sense to me.

Most predicates which access files need to cope with the possibility
that their target is deleted in between the time that the directory
entry data was read (e.g. by fts()) and the file is processed.  This
time window can be reasonably large, certainly large enough for some
other pcess to delete the file.

As regards handling the current directory, this has latterly become
rather complex. You need to use this kind of approach these days:

struct access_check_args
{
 const char *filename;
 int access_type;
 int cb_errno;
};


static int
access_callback(void *context)
{
 int rv;
 struct access_check_args *args = context;
 if ((rv = access(args->filename, args->access_type)) < 0)
   args->cb_errno = errno;
 return rv;
}

static int
can_access(int access_type)
{
 struct access_check_args args;
 args.filename = state.rel_pathname;
 args.access_type = access_type;
 args.cb_errno = 0;
 return 0 == run_in_dir(state.cwd_dir_fd, access_callback, &args);
}

boolean
pred_writable (const char *pathname, struct stat *stat_buf, struct
predicate *pred_ptr)
{
 (void) pathname;
 (void) stat_buf;
 (void) pred_ptr;

 return can_access(W_OK);
}


The reason for this is that find may or may not change CWD as it goes,
depending on which binary you are running and how it is configured
(e.g. the value of ftsoptions in ftsfind.c, the availability of
/proc/N/fd, and whether you are running the "find" or "oldfind"
binary).

The hacking with run_in_dir() is not always necessary if the OS or
gnulib provides a *at() function, like fstatat().   But the pathname
you pass to such functions in any case needs to be state.rel_pathname.

There are other reasons also for this difference between
state.relpathname and the pathname argument given to predicate
functions.  In the arbitrary case, it will be impossible to stat the
pathname passed to the predicate function, even if the CWD is the same
as find's starting point, because that pathname will likely be too
long (sometimes) for the kernel's PATH_MAX limit.

James.



> Anyway, I hope you're content with my work.  If you aren't at any time,
> please say so.
>
>
>   All the best and a nice weekend to you,
>
>     Leslie
>
>
> [1] 
> http://repo.or.cz/w/findutils.git?a=blobdiff;f=find/pred.c;h=1b1693f922ce3ab0428dfcb658d78c459cfb1143;hp=b3672a899ddb03dbfb71ba9f7e9f9510a4265c8a;hb=749482935e3a943a3c998f0786c0a9083a9f1940;hpb=2c907ecf38e6bc73bd6522fa2eb65dfec971bc8b
>
> --
> Personal homepage: https://viridian.dnsalias.net/~sky/homepage/
> gpg --keyserver pgp.mit.edu --recv-keys DD4EBF83