| Message ID | 20230328-nolibc-printf-test-v1-2-d7290ec893dd@weissschuh.net (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | tools/nolibc: add testcases for vfprintf | expand |
Hi Thomas, On Tue, Mar 28, 2023 at 09:01:30PM +0000, Thomas Weißschuh wrote: > This enables the usage of the stream APIs with arbitrary filedescriptors. > > It will be used by a future testcase. > Users can also use nolibc-specific code to do the same. > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> > --- > tools/include/nolibc/stdio.h | 36 +++++++----------------------------- > 1 file changed, 7 insertions(+), 29 deletions(-) > > diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h > index 96ac8afc5aee..cb58912b98e5 100644 > --- a/tools/include/nolibc/stdio.h > +++ b/tools/include/nolibc/stdio.h > @@ -21,17 +21,13 @@ > #define EOF (-1) > #endif > > -/* just define FILE as a non-empty type */ > typedef struct FILE { > - char dummy[1]; > + int fd; > } FILE; In my opinion this makes the usage of FILE* more complicated than before. Look for example at your vfprintf() test case, you had to do this with the fd provided by memfd_create(): w = vfprintf(&(FILE) { fd }, fmt, args); This is particularly ugly especially for code that needs to be exposed to end-user. Also it breaks compatibility with glibc that is seldom used to check if trouble comes from nolibc or from the test itself. It would be much better to have fdopen() here but the new struct makes this impossible. I would propose instead to go back to the previous definition and simply change its semantics a little bit: /* just define FILE as a non-empty type. The value of the pointer gives * the FD: FILE=~fd for fd>=0 or NULL for fd<0. This way positive FILE * are immediately identified as abnormal entries (i.e. possible copies * of valid pointers to something else). typedef struct FILE { char dummy[1]; } FILE; Then we can have: static __attribute__((unused)) FILE* const stdin = (FILE*)(uintptr_t)~STDIN_FILENO; static __attribute__((unused)) FILE* const stdout = (FILE*)(uintptr_t)~STDOUT_FILENO; static __attribute__((unused)) FILE* const stderr = (FILE*)(uintptr_t)~STDERR_FILENO; /* provides a FILE* equivalent of fd. The mode is ignored. */ static __attribute__((unused)) FILE *fdopen(int fd, const char *mode) { if (fd < 0) return NULL; return (FILE*)(uintptr_t)~fd; } And your FD can simply be passed this way: fd = memfd_create("vfprintf", 0); if (fd == -1) { pad_spc(llen, 64, "[FAIL]\n"); return 1; } va_start(args, fmt); w = vfprintf(fdopen(fd, "w+"), fmt, args); va_end(args); This way it works more like common usage and restores glibc compatibility. Regards, Willy
On 2023-04-02 09:45:57+0200, Willy Tarreau wrote: > Hi Thomas, > > On Tue, Mar 28, 2023 at 09:01:30PM +0000, Thomas Weißschuh wrote: > > This enables the usage of the stream APIs with arbitrary filedescriptors. > > > > It will be used by a future testcase. > > Users can also use nolibc-specific code to do the same. > > > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> > > --- > > tools/include/nolibc/stdio.h | 36 +++++++----------------------------- > > 1 file changed, 7 insertions(+), 29 deletions(-) > > > > diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h > > index 96ac8afc5aee..cb58912b98e5 100644 > > --- a/tools/include/nolibc/stdio.h > > +++ b/tools/include/nolibc/stdio.h > > @@ -21,17 +21,13 @@ > > #define EOF (-1) > > #endif > > > > -/* just define FILE as a non-empty type */ > > typedef struct FILE { > > - char dummy[1]; > > + int fd; > > } FILE; > > In my opinion this makes the usage of FILE* more complicated than before. > Look for example at your vfprintf() test case, you had to do this with > the fd provided by memfd_create(): > > w = vfprintf(&(FILE) { fd }, fmt, args); > > This is particularly ugly especially for code that needs to be exposed > to end-user. Also it breaks compatibility with glibc that is seldom used > to check if trouble comes from nolibc or from the test itself. It would > be much better to have fdopen() here but the new struct makes this > impossible. My original reasoning was to avoiding to implement a version of fdopen() that does not properly check the "mode" argument. But rereading the manpage of fdopen() we don't actually need to check them. Also the goal was for this "API" to be completely opaque to the enduser and only used internally in nolibc(-test). But building the tests with glibc is indeed a great usecase. fdopen() it is. Also I think I'll extend the -std=c99/-std=gnu89 series to also build the test against glibc to prevent me from breaking this in the future. > I would propose instead to go back to the previous definition and simply > change its semantics a little bit: > > /* just define FILE as a non-empty type. The value of the pointer gives > * the FD: FILE=~fd for fd>=0 or NULL for fd<0. This way positive FILE > * are immediately identified as abnormal entries (i.e. possible copies > * of valid pointers to something else). > typedef struct FILE { > char dummy[1]; > } FILE; > > Then we can have: > > static __attribute__((unused)) FILE* const stdin = (FILE*)(uintptr_t)~STDIN_FILENO; > static __attribute__((unused)) FILE* const stdout = (FILE*)(uintptr_t)~STDOUT_FILENO; > static __attribute__((unused)) FILE* const stderr = (FILE*)(uintptr_t)~STDERR_FILENO; > > /* provides a FILE* equivalent of fd. The mode is ignored. */ > static __attribute__((unused)) > FILE *fdopen(int fd, const char *mode) > { > if (fd < 0) > return NULL; > return (FILE*)(uintptr_t)~fd; > } > > And your FD can simply be passed this way: > > fd = memfd_create("vfprintf", 0); > if (fd == -1) { > pad_spc(llen, 64, "[FAIL]\n"); > return 1; > } > > va_start(args, fmt); > w = vfprintf(fdopen(fd, "w+"), fmt, args); > va_end(args); > > This way it works more like common usage and restores glibc compatibility. Ack, will do.
diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h index 96ac8afc5aee..cb58912b98e5 100644 --- a/tools/include/nolibc/stdio.h +++ b/tools/include/nolibc/stdio.h @@ -21,17 +21,13 @@ #define EOF (-1) #endif -/* just define FILE as a non-empty type */ typedef struct FILE { - char dummy[1]; + int fd; } FILE; -/* We define the 3 common stdio files as constant invalid pointers that - * are easily recognized. - */ -static __attribute__((unused)) FILE* const stdin = (FILE*)-3; -static __attribute__((unused)) FILE* const stdout = (FILE*)-2; -static __attribute__((unused)) FILE* const stderr = (FILE*)-1; +static __attribute__((unused)) FILE* const stdin = &(FILE){ STDIN_FILENO }; +static __attribute__((unused)) FILE* const stdout = &(FILE){ STDOUT_FILENO }; +static __attribute__((unused)) FILE* const stderr = &(FILE){ STDERR_FILENO }; /* getc(), fgetc(), getchar() */ @@ -41,14 +37,8 @@ static __attribute__((unused)) int fgetc(FILE* stream) { unsigned char ch; - int fd; - if (stream < stdin || stream > stderr) - return EOF; - - fd = 3 + (long)stream; - - if (read(fd, &ch, 1) <= 0) + if (read(stream->fd, &ch, 1) <= 0) return EOF; return ch; } @@ -68,14 +58,8 @@ static __attribute__((unused)) int fputc(int c, FILE* stream) { unsigned char ch = c; - int fd; - if (stream < stdin || stream > stderr) - return EOF; - - fd = 3 + (long)stream; - - if (write(fd, &ch, 1) <= 0) + if (write(stream->fd, &ch, 1) <= 0) return EOF; return ch; } @@ -96,15 +80,9 @@ static __attribute__((unused)) int _fwrite(const void *buf, size_t size, FILE *stream) { ssize_t ret; - int fd; - - if (stream < stdin || stream > stderr) - return EOF; - - fd = 3 + (long)stream; while (size) { - ret = write(fd, buf, size); + ret = write(stream->fd, buf, size); if (ret <= 0) return EOF; size -= ret;
This enables the usage of the stream APIs with arbitrary filedescriptors. It will be used by a future testcase. Users can also use nolibc-specific code to do the same. Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> --- tools/include/nolibc/stdio.h | 36 +++++++----------------------------- 1 file changed, 7 insertions(+), 29 deletions(-)