Front page | perl.perl5.porters |
Postings from September 2010
On Tue Dec 29 07:20:20 2009, alex.davies@talktalk.net wrote:
> > Here's a patch that removes an unnecesary call to win32_stat() by
> > the opendir code on Win32. This provides a noticeable speed up when
> > recursively traversing a directory eg. calls to File::Find::find().
> >
> > Note it does change behaviour in the following cases:
> >
> > 1. The patch makes directory names longer than MAX_PATH fail and
> > sets errno to ENAMETOOLONG ("Filename too long"). Currently, in
> > this case errno is not actually set in win32_opendir. Consider:
> >
> > c:\> perl -e "$!=0 ; opendir C, ('Q'x300) or die $!
> > Bad file descriptor at -e line 1.
> >
> > c:\> perl -e "$!=11 ; opendir C, ('Q'x300) or die $!
> > Resource temporarily unavailable at -e line 1.
That’s certainly a bug. I’m all for fixing that.
> > Note that when $!=0, pp_open_dir() has to resort to setting errno
> > to EBADF. This looks like an oversight in the original code as I
> > doubt this was the intended error code in this case.
> >
> > 2. The patch makes a zero length directory name fail and sets errno
> > to EINVAL. Currently "" is passed to win32_stat which happens to set
> > errno to ENOENT (i guess that's consistent over all Windows
> versions).
> > This looks like an oversight - why stat a filename that cannot
> possibly
> > exist? ie. setting errno to ENOENT here seems accidental. EINVAL
> > makes more sense. (I can't find a "Filename too short" error code ;-
> )
In this case, you will be making Windows different from Unix systems in
yet another case. On Mac OS X I get:
$ perl -le' opendir C, "" or die $!'
No such file or directory at -e line 1.
I think Windows should do the same.
> > In the above 2 cases, it's straightforward to change the patch code
> > to maintain backward compatibility. I don't have any great
> > attachment to changing the error codes and if someone wants to
> > argue for backwards compatibility i'll not haggle.
> >
> > Probably of more interest is...
> >
> > 3. If the directory is actually a regular file then currently errno
> > is not set, and so (as in case 1) errno ends up being set to EBADF.
> > The patched version will 'fail' via the same code path as if it's
> > just a non existant file/directory name (FindFirstFile will return
> > ERROR_PATH_NOT_FOUND) and so errno gets set to ENOENT.
> >
> > In this case both error codes are plainly wrong: there's
> > no file descriptor to be bad, and since it *is* a file it cannot
> > be said to be "No such file or directory".
> > Much better would be ENOTDIR ("Not a directory").
Agreed.
> > [Changing the patched version to make a distinction between where
> the
> > directory to be opened is neither-a-regular-file-nor-directory
> (ENOENT)
> > and is is-not-a-directory-(because-it's-a-regular-file) (ENOTDIR)
> would
> > require adding a stat in the ERROR_PATH_NOT_FOUND case.
> > Or alternatively just use ENOTDIR for both cases - but this would
> > expose a change in error code in the common case of
> > neither-a-regular-file-nor-directory.]
It would bring it into line with Unix, which I think would be good, but
not necessary for getting this patch through.
Could you resubmit the patch without the ‘Filename too short’ change?
Thread Previous