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

[Paul Eggert]

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.