Move from readdir_r to readdir #1955
Conversation
There was a problem hiding this comment.
This doesn't make any sense, according to the definition of errno any function call is free to modify it at will, the value is only guaranteed to carry any meaning after a function returns signalling that an error has occurred.
So saving and restoring a previous errno value should never be necessary, no code should depend on such behaviour.
| if (res) { | ||
| errno = 0; | ||
| entry = readdir(dir); | ||
| if (errno) { |
There was a problem hiding this comment.
The standard does actually not guarantee that a call to readdir() that returns a non-NULL pointer doesn't also modify errno, that guarantee is only specified for a call that returns NULL. So checking errno before checking the return value could possibly yield a false positive.
A safer implementation would be:
if (entry == NULL) {
return errno == 0;
}
|
Thanks Fredrik!
But I trust your judgment. |
| } | ||
| if (dir_result == NULL) { | ||
| return true; | ||
| errno = 0; |
There was a problem hiding this comment.
Given Fredrik's comment that this is slightly unobvious behaviour, can you add a comment explaining this here.
Setting errno explicitly to success before a call is not something I'm used to seeing.
There was a problem hiding this comment.
Fredrik's comment was on an outdated version of the file, I changed it. This is what he had suggested and if IIUC he doesn't think this one's unobvious - what he didn't think I should do was save the previous value of errno and set back to it if I come back with 0, which is what I had at commit 53642bf
Maybe let's discuss this in person. I'm happy with implementing it any way since you both know more here, but I'd like to understand it.
|
No worries, I read up some more and found http://pubs.opengroup.org/onlinepubs/009695399/functions/readdir.html and my questions are answered. This is the recommended way to use readdir, and I did add a comment to explain. |
b/31256592
Fixes #1307
Relevant man pages:
https://linux.die.net/man/3/readdir
https://linux.die.net/man/3/errno