Onderwerp: Re: [Full-disclosure] Re: readdir_r considered harmful

>Then you never really understood the implementation, seems. Of course
>all implementations keep the content of the directory as read with
>getdents or so in the DIR descriptor. But it is usually not the case
>that the whole content fits into the buffer allocated. One could, of
>course, resize the buffer to fit the content of the directory read,
>even if this means reserving hundreds or thousands of kBs. But this
>is not how most implementations work.

I don't see how that is relevant; the typical use of readdir() is as follows:

DIR *dirp = opendir(name);

while ((dent = readdir(dirp)) != NULL) {
...
}

closedir(dirp);

Nothing other threads do with readdir() on different dirp's will influence
what "dent" points to.

I have *never* seen a program where multiple threads read from a single
dirp; and I can't image the use.

>Instead implementations keep work similar to every buffered file I/O
>operation. But this means that buffer content is replaced. If this
>happens and some thread uses readdir() instead of readdir_r(), the
>returned string pointer suddenly becomes invalid since it points to
>memory which has been replaced.

Yes, the next call to readdir() *on the same dirp* may change what
the previous call; but that's completely irrelevant for most uses
of readdir().

Of course, an application may want to save all readdir() return values,
but that is completely orthogonal to threads; there is no reason
why the POSIX *thread* specification includes readdir_r().

>Next time, before you make such comments, ask Don Cragun to explain
>things to you.

Next time before you mail, you might want to engage your brain.

There is NO reason for a thread-safe library to use readdir_r() over
readdir(), with common readdir() implementations.

Casper

T24gMTEvNi8wNSwgQ2FzcGVyLkRpa0BzdW4uY29tIDxDYXNwZX IuRGlrQHN1bi5jb20+IHdyb3Rl
Ogo+IEkgZG9uJ3Qgc2VlIGhvdyB0aGF0IGlzIHJlbGV2YW50Oy B0aGUgdHlwaWNhbCB1c2Ugb2Yg
cmVhZGRpcigpIGlzIGFzIGZvbGxvd3M6Cj4KPiAgICAgICAgIE RJUiAqZGlycCA9IG9wZW5kaXIo
bmFtZSk7Cj4KPiAgICAgICAgIHdoaWxlICgoZGVudCA9IHJlYW RkaXIoZGlycCkpICE9IE5VTEwp
IHsKPiAgICAgICAgICAgICAgICAgLi4uCj4gICAgICAgICB9Cj 4KPiAgICAgICAgIGNsb3NlZGly
KGRpcnApOwo+Cj4gTm90aGluZyBvdGhlciB0aHJlYWRzIGRvIH dpdGggcmVhZGRpcigpIG9uIGRp
ZmZlcmVudCBkaXJwJ3Mgd2lsbCBpbmZsdWVuY2UKPiB3aGF0IC JkZW50IiBwb2ludHMgdG8uCgpU
aGUgaXNzdWUgaXMgbXVsdGlwbGUgdGhyZWFkcyB1c2luZyB0aG Ugc2FtZSBESVIuCg==

>On 11/6/05, Casper.Dik@sun.com <Casper.Dik@sun.com> wrote:
>> I don't see how that is relevant; the typical use of readdir() is as follows:
>>
>> DIR *dirp = opendir(name);
>>
>> while ((dent = readdir(dirp)) != NULL) {
>> ...
>> }
>>
>> closedir(dirp);
>>
>> Nothing other threads do with readdir() on different dirp's will influence
>> what "dent" points to.
>
>The issue is multiple threads using the same DIR.

No, it isn't. I certainly limited the scope of my contribution to
single threads reading from a DIR.

All the 80-odd uses of readdir_r() in the Solaris core source code,
all can (and should) be replaced with readdir(). All have a single
thread reading and reusing the same "struct dirent", so readdir()
could be used in POSIXly correct fashion if the following sentence
in the open group's manual page was not present:

"The readdir() function need not be reentrant. A function that is not
required to be reentrant is not required to be thread-safe."

I believe that this is an error in POSIX; when "threadedness" was added
the manual page could have been changed to indicate that a single
thread using the above idiom was safe.

Had they done so, we would never have had to use readdir_r() and progammers
would not have introduced bugs in the (mis)use of pathconf, over allocating,
etc.

I would be interested in seeing any real-world use of readdir_r() in
a context where readdir_r() is required (multiple threads reading from
a single DIR *).

Casper

--Apple-Mail-2-546709937
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset=US-ASCII; delsp=yes; format=flowed

On 06 Nov 05, at 01:00, Casper.Dik@Sun.COM wrote:
>> Then you never really understood the implementation, seems. Of
>> course
>> all implementations keep the content of the directory as read with
>> getdents or so in the DIR descriptor. But it is usually not the case
>> that the whole content fits into the buffer allocated. One could, of
>> course, resize the buffer to fit the content of the directory read,
>> even if this means reserving hundreds or thousands of kBs. But this
>> is not how most implementations work.
>>
>
> I don't see how that is relevant; the typical use of readdir() is
> as follows:
>
> DIR *dirp = opendir(name);
>
> while ((dent = readdir(dirp)) != NULL) {
> ...
> }
>
> closedir(dirp);
>
> Nothing other threads do with readdir() on different dirp's will
> influence
> what "dent" points to.
>
> I have *never* seen a program where multiple threads read from a
> single
> dirp; and I can't image the use.
>

In practice, you're correct. In theory, however, consider the
following code
path.


> THREAD 1 THREAD 2
> ------------------------------ ------------------------------
> DIR *d1 = opendir(dir1);
> DIR *d2 = opendir(dir2);
> dent1 = readdir(dir1);
> dent2 = readdir(dir2);
> use(dent1);
>

In most implementations, dent1 != dent2. HOWEVER, there is no
guarantee that
they will not both point to the same statically allocated buffer, and
some
implementations may do so. For example, this is why ctime_r exists:
ctime
returns a pointer to a statically allocated buffer, and hence is not
thread
safe.

You are correct, though, that the glibc implementation of readdir is
thread-safe, so readdir_r is unnecessary in all common situations.

--Apple-Mail-2-546709937
content-type: application/pgp-signature; x-mac-type=70674453;
name=PGP.sig
content-description: This is a digitally signed message part
content-disposition: inline; filename=PGP.sig
content-transfer-encoding: 7bit

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (Darwin)

iD8DBQFDbqdBPa6RRaKl0ScRAjwiAKDrmWxQLpuxnqdlC7LW40 1ro2NNpQCfZBQ7
sSW/1yS3FZ0wZOOFOediJP8=
=744w
-----END PGP SIGNATURE-----

--Apple-Mail-2-546709937--

>In practice, you're correct. In theory, however, consider the
>following code
>path.
>
>
>> THREAD 1 THREAD 2
>> ------------------------------ ------------------------------
>> DIR *d1 = opendir(dir1);
>> DIR *d2 = opendir(dir2);
>> dent1 = readdir(dir1);
>> dent2 = readdir(dir2);
>> use(dent1);
>>
>
>In most implementations, dent1 != dent2. HOWEVER, there is no
>guarantee that
>they will not both point to the same statically allocated buffer, and
>some
>implementations may do so. For example, this is why ctime_r exists:
>ctime
>returns a pointer to a statically allocated buffer, and hence is not
>thread
>safe.

The standard actually guarantees that the static storage is
associated with the specific directory STREAM. So a system on which
dent1 and dent2 point to the same buffer and reads from one stream
affect the buffer returned by reads from another stream are not
POSIX compliant.

See:

http://www.opengroup.org/onlinepubs/...s/readdir.html

"The pointer returned by readdir() points to data which may be
overwritten by another call to readdir() on the same directory
stream. This data is not overwritten by another call to readdir()
on a different directory stream."

But is also goes on to say:

"The readdir() function need not be reentrant. A function that is
not required to be reentrant is not required to be thread-safe."

which is the one thing I like POSIX to fix for thread safe implementations.

Casper

Casper.Dik@Sun.COM wrote:
....

>Had they done so, we would never have had to use readdir_r() and progammers
>would not have introduced bugs in the (mis)use of pathconf, over allocating,
>etc.
>
>I would be interested in seeing any real-world use of readdir_r() in
>a context where readdir_r() is required (multiple threads reading from
>a single DIR *).
>
>
Consider the following situation(I'm not sure if anyone actually does this):
1) You have a "spool" directory containing a large number of files, each
which represents a task to process.
2) You have a number of worker threads. Each worker thread reads a file
from the global DIR*, and then opens and reads the file for
processing(and then loops on 2).

Of course, you could always just put a mutex around every call to
readdir(), and copy the filename somewhere safe, or you could invent a
signalling system to ask one thread to do all the readdir()s. Whether
this makes sense depends on how much of readdir_r has to be spent inside
a global mutex/spinlock anyway, and how long the processing part takes
compared with the readdir() part.

Andrew