On Thu, Jan 30, 2025 at 08:54:03PM +0100, Thomas Weißschuh wrote:
> From: Thomas Weißschuh <thomas.weissschuh@xxxxxxxxxxxxx>
>
> Add an allocation-free implementation of readdir() and related
> functions. The implementation is modelled after the one for FILE.
I think you'd need to mention/remind the two important points that
come out of that choice, one being that DIR is a fake pointer that
instead stores ~fd so that it can be turned back to a valid FD, and
that subsequent readdir() calls will only work from the same file
unit since it relies on a local static storage.
Better have this visible in the commit message so that in the event
someone faces a difficulty due to this, they can easily find that it's
an on-purpose design choice.
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@xxxxxxxxxxxxx>
> Signed-off-by: Thomas Weißschuh <linux@xxxxxxxxxxxxxx>
>
> ---
> I'm not entirely sure where to put it. It doesn't really belong into
> stdio.h, but that's where the FILE stuff is.
> sys.h wants alphabetical ordering, but IMO these functions should stick
> together.
My man pages suggest that userland code will include <dirent.h>, thus
I think it could be the moment to create it with that new code.
> diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
> index 3892034198dd566d21a5cc0a9f67cf097d428393..1f275a0a7b6b2c6f1c15405d027c282bb77aa618 100644
> --- a/tools/include/nolibc/stdio.h
> +++ b/tools/include/nolibc/stdio.h
(...)
> +static __attribute__((unused))
> +struct dirent *readdir(DIR *dirp)
> +{
> + static struct dirent dirent;
> +
> + char buf[sizeof(struct linux_dirent64) + NAME_MAX];
I'm uncertain where NAME_MAX is defined, I haven't found it in the
nolibc sources, just double-checking that it's not just in your build
environment by accident.
> + struct linux_dirent64 *ldir = (void *)buf;
> + intptr_t i = (intptr_t)dirp;
> + int fd, ret;
> +
> + if (i >= 0) {
> + SET_ERRNO(EBADF);
> + return NULL;
> + }
> +
> + fd = ~i;
> +
> + ret = getdents64(fd, ldir, sizeof(buf));
> + if (ret == -1 || ret == 0)
> + return NULL;
> +
> + /*
> + * getdents64() returns as many entries as fit the buffer.
> + * readdir() can only return one entry at a time.
> + * Make sure the non-returned ones are not skipped.
> + */
> + ret = lseek(fd, ldir->d_off, SEEK_SET);
> + if (ret == -1)
> + return NULL;
> +
> + dirent = (struct dirent) {
> + .d_ino = ldir->d_ino,
> + };
> + strlcpy(dirent.d_name, ldir->d_name, sizeof(dirent.d_name));
Just out of curiosity, could this copy fail, and if so, should we handle
it (e.g. NAME_MAX != 255) ? My guess here is that if it could almost never
fail and checking it would needlessly complicate the function, let's just
handle it with a comment for now. And if it cannot at all, let's mention
why on top of it as well.
Thanks,
Willy
