This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v2] posix: Fix glob with GLOB_NOCHECK returning modified patterns (BZ#10246)


Adhemerval Zanella wrote:

Without actually following the possible directory the obtained
matched name the example you provided:

   ln -s /no-such-file globlink1
   ln -s . globlink2

Will be matched only for globlink1.

You're correct that there's a bug here, that needs to be fixed. But the proposed patch changes the semantics of GLOB_ONLYDIR contrary to its intent. GLOB_ONLYDIR is supposed to be an efficiency flag, to let callers run faster. By making GLOB_ONLYDIR run slower, the patch is contrary to this expectation.

The bug needs to be fixed, but not this way. It's the caller's responsibility to check that the returned value is indeed a directory, and that's how the fix should be written: by fixing the caller to check that the result is indeed a directory, not by slowing down the callee.

Also, if readdir_result_type (d) is DT_UNKNOWN, d.name might be a directory, so the code should check in that case too.

I excluded DT_UNKNOWN mainly because posix/tst-gnuglob.c uses a for the synthetic
directory to tests some entries as DT_UNKNOWN and it does not expect them to
be matches.

That sounds like a bug in the test case, then. DT_UNKNOWN is returned for file systems that do not have the file type in the inode. A DT_UNKNOWN entry might be a directory, or a symlink, or anything else.

If we change to also match DT_UNKNOWN for possible directories
entries it would meant change slight current glob semantic.

How so? DT_UNKNOWN merely means that glob can't do some optimizations. This should not change the semantics, only performance.

Also, d.name is just a file name component: it should be absolute, or relative to the working directory.

Not really sure what this may lead to issue in current patch, care to
elaborate?

As I recall, d.name is relative to the parent directory: it has no slashes. The argument to is_dir is supposed to be the full filename, either absolute or relative to the current working directory.

Also, the code mistakenly treats leading "//" as if it were "/"; this is not correct, and does not work on some platforms. POSIX allows implementations where "//" is distinct from "/".

Indeed, although for *glibc* supported platforms this is not an issue.

Fair enough, we can fix that separately.

Using either 'globlink[12]//' or 'globlink[12]///' will handle the same
results as 'globlink[12]/'.

Every result returned by globlink[12]/// should end in three slashes. The number of slashes in the result should equal the number of slashes in the pattern. This general issue differs from the issue about // vs / at the start of the file name, as the trailing slashes are portable to all filesystems.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]