[gdal-dev] test failures on pkgsrc build of 3.3.2rc3

[Greg Troxel]

Even Rouault <even.rouault at spatialys.com> writes:

>> As I read the spec, it is a violation to return NULL when the first
>> argument is a directory and the second is r or rb.  A test program
>> succeeds in calling fopen on . with rb, on both NetBSD and macOS 10.13.
>
> It is not clear for me. There's a mention that opening a directory in
> update mode should fail.

Directories are not writable as files, even if they have the w bit.
That's been true since at least Sixth Edition.  To me, that text
prohibiting opening directories for write is clear evidence of intent
not to prohibit opening them for reading.

> But I can't see the point of succeeding in read-only mode. You can't
> read anything.

Directories have names and inode numbers, in some data structure.  There
is something to read -- it's just something that usually one does not
want to do.

I agree that in the modern world trying to read a directory is not
really a sensible thing to do under almost all circumstances.  But it's
a huge leap from that observation to assuming that attempts must fail.

> But actually I do see that on my current Linux system too
> fopen(somedir, "rb") works, but later in that function, we try to read
> some bytes from it. If we don't manage to read a single byte, then we
> fallback to a stat() to check the nature of the file. So I suspect
> that on your OS and FreeBSD, the fread() does return some bytes,
> whereas the other OS on which we regularly test, return 0.

Interesting that this is even more subtle than I thought.  Still, I
don't think POSIX says that fread on a directory can't return bytes.  In
fact I can't find anything in fread or fgetc that allows it to fail on
directories.  I suppose though that a system can define a directory to
have zero bytes of content when viewed as a file.

On NetBSD, bytes are returned, and I see filenames.  The output from
fread matches "od -c .".

> I guess we might want to change the current test as #if
> !(defined(_WIN32) || defined(__linux__) || defined(__ANDROID__)) so as
> to match all the BSDs, and do the stat() before trying to fopen() the
> file.
>
> Trying that in https://github.com/OSGeo/gdal/pull/4411

I don't understand why there should be an explicit list of systems when
the basic issue is code relying on behavior that is not specified by
POSIX.  From a traditional UNIX perspective, I find fread refusing to
read a directory to be strange -- but at the same time it's a reasonable
choice from an abstraction viewpoint.

It seems far safer to just not make assumptions about what fopen/fread
will do, when one can't say a system that doesn't meet the assumptions
is defective.

What's wrong with just always doing the stat?

Or, if it's really important to avoid the stat, put the code that makes
beyond-POSIX assumptions under #ifdef linux.   Then we'd have code that
is in general correct, and avoids a stat on systems known to not return
any bytes on a directory.

> This will be post 3.3.2 material as it is not a new issue.

Sure, that's fine.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 194 bytes
Desc: not available
URL: <http://lists.osgeo.org/pipermail/gdal-dev/attachments/20210901/cdb72852/attachment.sig>