* [PATCH] Linux: Do not skip d_ino == 0 entries in readdir, readdir64 (bug 12165)
@ 2022-09-19 6:54 Florian Weimer
2022-09-19 8:26 ` Andreas Schwab
0 siblings, 1 reply; 6+ messages in thread
From: Florian Weimer @ 2022-09-19 6:54 UTC (permalink / raw)
To: libc-alpha
POSIX does not say this value is special. For example, old XFS file
systems may still use inode number zero.
Also update the comment regarding ENOENT. Linux may return ENOENT
for some file systems.
Tested on i686-linux-gnu and x86_64-linux-gnu.
---
sysdeps/unix/sysv/linux/readdir.c | 57 +++++++----------
sysdeps/unix/sysv/linux/readdir64.c | 120 +++++++++++++++---------------------
2 files changed, 69 insertions(+), 108 deletions(-)
diff --git a/sysdeps/unix/sysv/linux/readdir.c b/sysdeps/unix/sysv/linux/readdir.c
index b480135164..15f0033f36 100644
--- a/sysdeps/unix/sysv/linux/readdir.c
+++ b/sysdeps/unix/sysv/linux/readdir.c
@@ -28,48 +28,33 @@ __readdir_unlocked (DIR *dirp)
struct dirent *dp;
int saved_errno = errno;
- do
+ if (dirp->offset >= dirp->size)
{
- size_t reclen;
+ /* We've emptied out our buffer. Refill it. */
- if (dirp->offset >= dirp->size)
+ size_t maxread = dirp->allocation;
+ ssize_t bytes;
+
+ bytes = __getdents (dirp->fd, dirp->data, maxread);
+ if (bytes <= 0)
{
- /* We've emptied out our buffer. Refill it. */
-
- size_t maxread = dirp->allocation;
- ssize_t bytes;
-
- bytes = __getdents (dirp->fd, dirp->data, maxread);
- if (bytes <= 0)
- {
- /* On some systems getdents fails with ENOENT when the
- open directory has been rmdir'd already. POSIX.1
- requires that we treat this condition like normal EOF. */
- if (bytes < 0 && errno == ENOENT)
- bytes = 0;
-
- /* Don't modifiy errno when reaching EOF. */
- if (bytes == 0)
- __set_errno (saved_errno);
- dp = NULL;
- break;
- }
- dirp->size = (size_t) bytes;
-
- /* Reset the offset into the buffer. */
- dirp->offset = 0;
+ /* Linux fails with ENOENT if IS_DEADDIR is true for the
+ directory inode. POSIX treats this as a regular
+ end-of-directory condition, so do not set errno in that
+ case, to indicate success. */
+ if (bytes == 0 || errno == ENOENT)
+ __set_errno (saved_errno);
+ return NULL;
}
+ dirp->size = (size_t) bytes;
- dp = (struct dirent *) &dirp->data[dirp->offset];
+ /* Reset the offset into the buffer. */
+ dirp->offset = 0;
+ }
- reclen = dp->d_reclen;
-
- dirp->offset += reclen;
-
- dirp->filepos = dp->d_off;
-
- /* Skip deleted files. */
- } while (dp->d_ino == 0);
+ dp = (struct dirent *) &dirp->data[dirp->offset];
+ dirp->offset += dp->d_reclen;
+ dirp->filepos = dp->d_off;
return dp;
}
diff --git a/sysdeps/unix/sysv/linux/readdir64.c b/sysdeps/unix/sysv/linux/readdir64.c
index 52b11eb9d9..f56c3c0d8e 100644
--- a/sysdeps/unix/sysv/linux/readdir64.c
+++ b/sysdeps/unix/sysv/linux/readdir64.c
@@ -37,48 +37,36 @@ __readdir64 (DIR *dirp)
__libc_lock_lock (dirp->lock);
#endif
- do
+ if (dirp->offset >= dirp->size)
{
- size_t reclen;
+ /* We've emptied out our buffer. Refill it. */
- if (dirp->offset >= dirp->size)
+ size_t maxread = dirp->allocation;
+ ssize_t bytes;
+
+ bytes = __getdents64 (dirp->fd, dirp->data, maxread);
+ if (bytes <= 0)
{
- /* We've emptied out our buffer. Refill it. */
-
- size_t maxread = dirp->allocation;
- ssize_t bytes;
-
- bytes = __getdents64 (dirp->fd, dirp->data, maxread);
- if (bytes <= 0)
- {
- /* On some systems getdents fails with ENOENT when the
- open directory has been rmdir'd already. POSIX.1
- requires that we treat this condition like normal EOF. */
- if (bytes < 0 && errno == ENOENT)
- bytes = 0;
-
- /* Don't modifiy errno when reaching EOF. */
- if (bytes == 0)
- __set_errno (saved_errno);
- dp = NULL;
- break;
- }
- dirp->size = (size_t) bytes;
-
- /* Reset the offset into the buffer. */
- dirp->offset = 0;
+ /* Linux fails with ENOENT if IS_DEADDIR is true for the
+ directory inode. POSIX treats this as a regular
+ end-of-directory condition, so do not set errno in that
+ case, to indicate success. */
+ if (bytes == 0 || errno == ENOENT)
+ __set_errno (saved_errno);
+#if IS_IN (libc)
+ __libc_lock_unlock (dirp->lock);
+#endif
+ return NULL;
}
+ dirp->size = (size_t) bytes;
- dp = (struct dirent64 *) &dirp->data[dirp->offset];
+ /* Reset the offset into the buffer. */
+ dirp->offset = 0;
+ }
- reclen = dp->d_reclen;
-
- dirp->offset += reclen;
-
- dirp->filepos = dp->d_off;
-
- /* Skip deleted files. */
- } while (dp->d_ino == 0);
+ dp = (struct dirent64 *) &dirp->data[dirp->offset];
+ dirp->offset += dp->d_reclen;
+ dirp->filepos = dp->d_off;
#if IS_IN (libc)
__libc_lock_unlock (dirp->lock);
@@ -115,48 +103,36 @@ __old_readdir64 (DIR *dirp)
__libc_lock_lock (dirp->lock);
#endif
- do
+ if (dirp->offset >= dirp->size)
{
- size_t reclen;
+ /* We've emptied out our buffer. Refill it. */
- if (dirp->offset >= dirp->size)
+ size_t maxread = dirp->allocation;
+ ssize_t bytes;
+
+ bytes = __old_getdents64 (dirp->fd, dirp->data, maxread);
+ if (bytes <= 0)
{
- /* We've emptied out our buffer. Refill it. */
-
- size_t maxread = dirp->allocation;
- ssize_t bytes;
-
- bytes = __old_getdents64 (dirp->fd, dirp->data, maxread);
- if (bytes <= 0)
- {
- /* On some systems getdents fails with ENOENT when the
- open directory has been rmdir'd already. POSIX.1
- requires that we treat this condition like normal EOF. */
- if (bytes < 0 && errno == ENOENT)
- bytes = 0;
-
- /* Don't modifiy errno when reaching EOF. */
- if (bytes == 0)
- __set_errno (saved_errno);
- dp = NULL;
- break;
- }
- dirp->size = (size_t) bytes;
-
- /* Reset the offset into the buffer. */
- dirp->offset = 0;
+ /* Linux fails with ENOENT if IS_DEADDIR is true for the
+ directory inode. POSIX treats this as a regular
+ end-of-directory condition, so do not set errno in that
+ case, to indicate success. */
+ if (bytes == 0 || errno == ENOENT)
+ __set_errno (saved_errno);
+#if IS_IN (libc)
+ __libc_lock_unlock (dirp->lock);
+#endif
+ return NULL;
}
+ dirp->size = (size_t) bytes;
- dp = (struct __old_dirent64 *) &dirp->data[dirp->offset];
+ /* Reset the offset into the buffer. */
+ dirp->offset = 0;
+ }
- reclen = dp->d_reclen;
-
- dirp->offset += reclen;
-
- dirp->filepos = dp->d_off;
-
- /* Skip deleted files. */
- } while (dp->d_ino == 0);
+ dp = (struct __old_dirent64 *) &dirp->data[dirp->offset];
+ dirp->offset += dp->d_reclen;
+ dirp->filepos = dp->d_off;
#if IS_IN (libc)
__libc_lock_unlock (dirp->lock);
base-commit: 2ff6775ad341b10a08e3b27d6e1df1da637747c7
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Linux: Do not skip d_ino == 0 entries in readdir, readdir64 (bug 12165)
2022-09-19 6:54 [PATCH] Linux: Do not skip d_ino == 0 entries in readdir, readdir64 (bug 12165) Florian Weimer
@ 2022-09-19 8:26 ` Andreas Schwab
2022-09-19 8:30 ` Florian Weimer
0 siblings, 1 reply; 6+ messages in thread
From: Andreas Schwab @ 2022-09-19 8:26 UTC (permalink / raw)
To: Florian Weimer via Libc-alpha; +Cc: Florian Weimer
On Sep 19 2022, Florian Weimer via Libc-alpha wrote:
> - if (bytes <= 0)
> - {
> - /* On some systems getdents fails with ENOENT when the
Shouldn't that be "some file systems"?
> - open directory has been rmdir'd already. POSIX.1
> - requires that we treat this condition like normal EOF. */
> - if (bytes < 0 && errno == ENOENT)
> - bytes = 0;
> -
> - /* Don't modifiy errno when reaching EOF. */
> - if (bytes == 0)
> - __set_errno (saved_errno);
> - dp = NULL;
> - break;
> - }
> - dirp->size = (size_t) bytes;
> -
> - /* Reset the offset into the buffer. */
> - dirp->offset = 0;
> + /* Linux fails with ENOENT if IS_DEADDIR is true for the
What is IS_DEADDIR?
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Linux: Do not skip d_ino == 0 entries in readdir, readdir64 (bug 12165)
2022-09-19 8:26 ` Andreas Schwab
@ 2022-09-19 8:30 ` Florian Weimer
2022-09-19 8:53 ` Andreas Schwab
0 siblings, 1 reply; 6+ messages in thread
From: Florian Weimer @ 2022-09-19 8:30 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Florian Weimer via Libc-alpha
* Andreas Schwab:
> On Sep 19 2022, Florian Weimer via Libc-alpha wrote:
>
>> - if (bytes <= 0)
>> - {
>> - /* On some systems getdents fails with ENOENT when the
>
> Shouldn't that be "some file systems"?
>
>> - open directory has been rmdir'd already. POSIX.1
>> - requires that we treat this condition like normal EOF. */
>> - if (bytes < 0 && errno == ENOENT)
>> - bytes = 0;
>> -
>> - /* Don't modifiy errno when reaching EOF. */
>> - if (bytes == 0)
>> - __set_errno (saved_errno);
>> - dp = NULL;
>> - break;
>> - }
>> - dirp->size = (size_t) bytes;
>> -
>> - /* Reset the offset into the buffer. */
>> - dirp->offset = 0;
>> + /* Linux fails with ENOENT if IS_DEADDIR is true for the
>
> What is IS_DEADDIR?
It's an internal Linux construct. Isn't this clear from the context?
Thanks,
Florian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Linux: Do not skip d_ino == 0 entries in readdir, readdir64 (bug 12165)
2022-09-19 8:30 ` Florian Weimer
@ 2022-09-19 8:53 ` Andreas Schwab
2022-09-19 9:30 ` Florian Weimer
0 siblings, 1 reply; 6+ messages in thread
From: Andreas Schwab @ 2022-09-19 8:53 UTC (permalink / raw)
To: Florian Weimer; +Cc: Florian Weimer via Libc-alpha
On Sep 19 2022, Florian Weimer wrote:
> * Andreas Schwab:
>
>> On Sep 19 2022, Florian Weimer via Libc-alpha wrote:
>>
>>> - if (bytes <= 0)
>>> - {
>>> - /* On some systems getdents fails with ENOENT when the
>>
>> Shouldn't that be "some file systems"?
>>
>>> - open directory has been rmdir'd already. POSIX.1
>>> - requires that we treat this condition like normal EOF. */
>>> - if (bytes < 0 && errno == ENOENT)
>>> - bytes = 0;
>>> -
>>> - /* Don't modifiy errno when reaching EOF. */
>>> - if (bytes == 0)
>>> - __set_errno (saved_errno);
>>> - dp = NULL;
>>> - break;
>>> - }
>>> - dirp->size = (size_t) bytes;
>>> -
>>> - /* Reset the offset into the buffer. */
>>> - dirp->offset = 0;
>>> + /* Linux fails with ENOENT if IS_DEADDIR is true for the
>>
>> What is IS_DEADDIR?
>
> It's an internal Linux construct. Isn't this clear from the context?
I'd rather keep the original comment with the missing word added. I
find it easier to understand without the reference to kernel internals.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Linux: Do not skip d_ino == 0 entries in readdir, readdir64 (bug 12165)
2022-09-19 8:53 ` Andreas Schwab
@ 2022-09-19 9:30 ` Florian Weimer
2022-09-19 9:58 ` Andreas Schwab
0 siblings, 1 reply; 6+ messages in thread
From: Florian Weimer @ 2022-09-19 9:30 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Florian Weimer via Libc-alpha
* Andreas Schwab:
> On Sep 19 2022, Florian Weimer wrote:
>
>> * Andreas Schwab:
>>
>>> On Sep 19 2022, Florian Weimer via Libc-alpha wrote:
>>>
>>>> - if (bytes <= 0)
>>>> - {
>>>> - /* On some systems getdents fails with ENOENT when the
>>>
>>> Shouldn't that be "some file systems"?
>>>
>>>> - open directory has been rmdir'd already. POSIX.1
>>>> - requires that we treat this condition like normal EOF. */
>>>> - if (bytes < 0 && errno == ENOENT)
>>>> - bytes = 0;
>>>> -
>>>> - /* Don't modifiy errno when reaching EOF. */
>>>> - if (bytes == 0)
>>>> - __set_errno (saved_errno);
>>>> - dp = NULL;
>>>> - break;
>>>> - }
>>>> - dirp->size = (size_t) bytes;
>>>> -
>>>> - /* Reset the offset into the buffer. */
>>>> - dirp->offset = 0;
>>>> + /* Linux fails with ENOENT if IS_DEADDIR is true for the
>>>
>>> What is IS_DEADDIR?
>>
>> It's an internal Linux construct. Isn't this clear from the context?
>
> I'd rather keep the original comment with the missing word added. I
> find it easier to understand without the reference to kernel internals.
What about this?
/* Linux may fail with ENOENT on some file systems if the
directory inode is marked as dead (deleted). POSIX
treats this as a regular end-of-directory condition, so
do not set errno in that case, to indicate success. */
Thanks,
Florian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Linux: Do not skip d_ino == 0 entries in readdir, readdir64 (bug 12165)
2022-09-19 9:30 ` Florian Weimer
@ 2022-09-19 9:58 ` Andreas Schwab
0 siblings, 0 replies; 6+ messages in thread
From: Andreas Schwab @ 2022-09-19 9:58 UTC (permalink / raw)
To: Florian Weimer; +Cc: Florian Weimer via Libc-alpha
On Sep 19 2022, Florian Weimer wrote:
> What about this?
>
> /* Linux may fail with ENOENT on some file systems if the
> directory inode is marked as dead (deleted). POSIX
> treats this as a regular end-of-directory condition, so
> do not set errno in that case, to indicate success. */
Ok.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-09-19 9:58 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-19 6:54 [PATCH] Linux: Do not skip d_ino == 0 entries in readdir, readdir64 (bug 12165) Florian Weimer
2022-09-19 8:26 ` Andreas Schwab
2022-09-19 8:30 ` Florian Weimer
2022-09-19 8:53 ` Andreas Schwab
2022-09-19 9:30 ` Florian Weimer
2022-09-19 9:58 ` Andreas Schwab
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).