From 476b27c301c2557615bdc80c45eb547a4cc8d6e7 Mon Sep 17 00:00:00 2001 From: Jonas 'Sortie' Termansen Date: Wed, 20 Mar 2013 22:12:20 +0100 Subject: [PATCH] Refactor FILE creation and destruction. --- libc/Makefile | 3 ++ libc/decl/FILE.h | 4 +- libc/fclose.cpp | 13 ++---- libc/fdeletefile.cpp | 32 ++++++++++++++ libc/fdio.c | 102 ++++++++++++++++++++++++++++--------------- libc/fdio.h | 7 ++- libc/fgets.cpp | 2 + libc/fnewfile.cpp | 18 +++----- libc/fresetfile.cpp | 46 +++++++++++++++++++ libc/fshutdown.cpp | 45 +++++++++++++++++++ libc/include/stdio.h | 3 ++ libc/stdio.c | 9 ++-- 12 files changed, 222 insertions(+), 62 deletions(-) create mode 100644 libc/fdeletefile.cpp create mode 100644 libc/fresetfile.cpp create mode 100644 libc/fshutdown.cpp diff --git a/libc/Makefile b/libc/Makefile index 84f746e0..b247f807 100644 --- a/libc/Makefile +++ b/libc/Makefile @@ -35,6 +35,7 @@ errno.o \ fabs.o \ fbufsize.o \ fclose.o \ +fdeletefile.o \ feof.o \ ferror.o \ fflush.o \ @@ -54,12 +55,14 @@ freadable.o \ freading.o \ fread.o \ fregister.o \ +fresetfile.o \ fscanf.o \ fseek.o \ fseeko.o \ fsetdefaultbuf.o \ fseterr.o \ fsetlocking.o \ +fshutdown.o \ ftell.o \ ftello.o \ fwritable.o \ diff --git a/libc/decl/FILE.h b/libc/decl/FILE.h index a5bad2e6..8504b200 100644 --- a/libc/decl/FILE.h +++ b/libc/decl/FILE.h @@ -18,6 +18,8 @@ typedef struct _FILE size_t buffersize; unsigned char* buffer; void* user; + void* free_user; + int (*reopen_func)(void* user, const char* mode); size_t (*read_func)(void* ptr, size_t size, size_t nmemb, void* user); size_t (*write_func)(const void* ptr, size_t size, size_t nmemb, void* user); int (*seek_func)(void* user, off_t offset, int whence); @@ -28,7 +30,7 @@ typedef struct _FILE int (*error_func)(void* user); int (*fileno_func)(void* user); int (*close_func)(void* user); - void (*free_func)(struct _FILE* fp); + void (*free_func)(void* free_user, struct _FILE* fp); /* Application writers shouldn't use anything beyond this point. */ struct _FILE* prev; struct _FILE* next; diff --git a/libc/fclose.cpp b/libc/fclose.cpp index 61405dfa..c8733a92 100644 --- a/libc/fclose.cpp +++ b/libc/fclose.cpp @@ -26,14 +26,7 @@ extern "C" int fclose(FILE* fp) { - if ( fflush(fp) ) - { - /* TODO: How to report errors here? fclose may need us to return its - exact error value, for instance, as with popen/pclose. */; - } - int result = fp->close_func ? fp->close_func(fp->user) : 0; - funregister(fp); - if ( fp->free_func ) - fp->free_func(fp); - return result; + int ret = fshutdown(fp); + fdeletefile(fp); + return ret; } diff --git a/libc/fdeletefile.cpp b/libc/fdeletefile.cpp new file mode 100644 index 00000000..cf14b440 --- /dev/null +++ b/libc/fdeletefile.cpp @@ -0,0 +1,32 @@ +/******************************************************************************* + + Copyright(C) Jonas 'Sortie' Termansen 2013. + + This file is part of the Sortix C Library. + + The Sortix C Library is free software: you can redistribute it and/or modify + it under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation, either version 3 of the License, or (at your + option) any later version. + + The Sortix C Library is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public + License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with the Sortix C Library. If not, see . + + fdeletefile.cpp + Deallocator for things returned by fnewfile after being shut down. + +*******************************************************************************/ + +#include + +extern "C" void fdeletefile(FILE* fp) +{ + funregister(fp); + if ( fp->free_func ) + fp->free_func(fp->free_user, fp); +} diff --git a/libc/fdio.c b/libc/fdio.c index db6fdb29..8b77b41d 100644 --- a/libc/fdio.c +++ b/libc/fdio.c @@ -45,6 +45,17 @@ typedef struct fdio_struct int fd; } fdio_t; +static int fdio_reopen(void* user, const char* mode) +{ + (void) user; + (void) mode; + // TODO: Unfortunately, we don't support this yet. Note that we don't really + // have to support this according to POSIX - but it'd be nicer to push this + // restriction into the kernel and argue it's a security problem "What? No + // you can't make this read-only descriptor readable!". + return errno = ENOTSUP, -1; +} + static size_t fdio_read(void* ptr, size_t size, size_t nmemb, void* user) { uint8_t* buf = (uint8_t*) ptr; @@ -57,6 +68,8 @@ static size_t fdio_read(void* ptr, size_t size, size_t nmemb, void* user) ssize_t numbytes = read(fdio->fd, buf + sofar, total - sofar); if ( numbytes < 0 ) { fdio->flags |= FDIO_ERROR; break; } if ( numbytes == 0 ) { fdio->flags |= FDIO_EOF; break; } + // TODO: Is this a bug? Looks like one, but perhaps this is needed when + // reading from line-buffered terminals. return numbytes / size; sofar += numbytes; } @@ -130,7 +143,33 @@ static int fdio_close(void* user) return result; } -int fdio_install(FILE* fp, const char* mode, int fd) +int fdio_open_descriptor(const char* path, const char* mode) +{ + int omode = 0; + int oflags = 0; + char c; + // TODO: This is too hacky and a little buggy. + const char* origmode = mode; + while ( (c = *mode++) ) + switch ( c ) + { + case 'r': omode = O_RDONLY; break; + case 'a': oflags |= O_APPEND; /* fall-through */ + case 'w': omode = O_WRONLY; oflags |= O_CREAT | O_TRUNC; break; + case '+': + omode = O_RDWR; + break; + case 'b': break; + case 't': break; + default: + fprintf(stderr, "Unsupported fopen mode: '%s'\n", origmode); + errno = EINVAL; + return -1; + } + return open(path, omode | oflags, 0666); +} + +int fdio_install_fd(FILE* fp, int fd, const char* mode) { fdio_t* fdio = (fdio_t*) calloc(1, sizeof(fdio_t)); if ( !fdio ) @@ -155,6 +194,7 @@ int fdio_install(FILE* fp, const char* mode, int fd) if ( !fstat(fd, &st) && fdio->flags & FDIO_WRITING && S_ISDIR(st.st_mode) ) return free(fdio), errno = EISDIR, 0; fp->user = fdio; + fp->reopen_func = fdio_reopen; fp->read_func = fdio_read; fp->write_func = fdio_write; fp->seek_func = fdio_seek; @@ -170,48 +210,42 @@ int fdio_install(FILE* fp, const char* mode, int fd) return 1; } -FILE* fdio_newfile(int fd, const char* mode) +int fdio_install_path(FILE* fp, const char* path, const char* mode) +{ + int fd = fdio_open_descriptor(path, mode); + if ( fd < 0 ) + return 0; + if ( !fdio_install_fd(fp, fd, mode) ) + return close(fd), 0; + return 1; +} + +FILE* fdio_new_fd(int fd, const char* mode) { FILE* fp = fnewfile(); - if ( !fp ) { return NULL; } - if ( !fdio_install(fp, mode, fd) ) { fclose(fp); return NULL; } + if ( !fp ) + return NULL; + if ( !fdio_install_fd(fp, fd, mode) ) + return fclose(fp), (FILE*) NULL; + return fp; +} + +FILE* fdio_new_path(const char* path, const char* mode) +{ + FILE* fp = fnewfile(); + if ( !fp ) + return NULL; + if ( !fdio_install_path(fp, path, mode) ) + return fclose(fp), (FILE*) NULL; return fp; } FILE* fdopen(int fd, const char* mode) { - return fdio_newfile(fd, mode); + return fdio_new_fd(fd, mode); } FILE* fopen(const char* path, const char* mode) { - int omode = 0; - int oflags = 0; - char c; - // TODO: This is too hacky and a little buggy. - const char* origmode = mode; - while ( ( c = *mode++ ) ) - { - switch ( c ) - { - case 'r': omode = O_RDONLY; break; - case 'a': oflags |= O_APPEND; /* fall-through */ - case 'w': omode = O_WRONLY; oflags |= O_CREAT | O_TRUNC; break; - case '+': - omode = O_RDWR; - break; - case 'b': break; - case 't': break; - default: - fprintf(stderr, "Unsupported fopen mode: '%s'\n", origmode); - errno = EINVAL; - return 0; - } - } - mode = origmode; - int fd = open(path, omode | oflags, 0666); - if ( fd < 0 ) { return NULL; } - FILE* fp = fdopen(fd, mode); - if ( !fp ) { close(fd); return NULL; } - return fp; + return fdio_new_path(path, mode); } diff --git a/libc/fdio.h b/libc/fdio.h index 3c46e019..caa0a3d5 100644 --- a/libc/fdio.h +++ b/libc/fdio.h @@ -29,8 +29,11 @@ __BEGIN_DECLS -int fdio_install(FILE* fp, const char* mode, int fd); -FILE* fdio_newfile(int fd, const char* mode); +int fdio_install_fd(FILE* fp, int fd, const char* mode); +int fdio_install_path(FILE* fp, const char* path, const char* mode); +FILE* fdio_new_fd(int fd, const char* mode); +FILE* fdio_new_path(const char* path, const char* mode); +int fdio_open_descriptor(const char* path, const char* mode); __END_DECLS diff --git a/libc/fgets.cpp b/libc/fgets.cpp index 3e0dbe74..869cbd58 100644 --- a/libc/fgets.cpp +++ b/libc/fgets.cpp @@ -40,6 +40,8 @@ extern "C" char* fgets(char* dest, int size, FILE* fp) } if ( !i && (ferror(fp) || feof(fp)) ) return NULL; + // TODO: The end-of-file state is lost here if feof(fp) and we are reading + // from a terminal that encountered a soft EOF. dest[i] = '\0'; return dest; } diff --git a/libc/fnewfile.cpp b/libc/fnewfile.cpp index 702804f4..3887a732 100644 --- a/libc/fnewfile.cpp +++ b/libc/fnewfile.cpp @@ -25,25 +25,19 @@ #include #include -static void ffreefile(FILE* fp) +static void fnewfile_destroyer(void* /*user*/, FILE* fp) { - if ( fp->flags & _FILE_BUFFER_OWNED ) - free(fp->buffer); free(fp); } extern "C" FILE* fnewfile(void) { FILE* fp = (FILE*) calloc(sizeof(FILE), 1); - if ( !fp ) { return NULL; } - fp->buffersize = 0; - fp->buffer = NULL; - fp->flags = _FILE_AUTO_LOCK; - fp->buffer_mode = 0; - fp->offset_input_buffer = 0; - fp->amount_input_buffered = 0; - fp->amount_output_buffered = 0; - fp->free_func = ffreefile; + if ( !fp ) + return NULL; + fp->free_user = NULL; + fp->free_func = fnewfile_destroyer; + fresetfile(fp); fregister(fp); return fp; } diff --git a/libc/fresetfile.cpp b/libc/fresetfile.cpp new file mode 100644 index 00000000..6ee9adf2 --- /dev/null +++ b/libc/fresetfile.cpp @@ -0,0 +1,46 @@ +/******************************************************************************* + + Copyright(C) Jonas 'Sortie' Termansen 2011, 2012, 2013. + + This file is part of the Sortix C Library. + + The Sortix C Library is free software: you can redistribute it and/or modify + it under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation, either version 3 of the License, or (at your + option) any later version. + + The Sortix C Library is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public + License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with the Sortix C Library. If not, see . + + fresetfile.cpp + After a FILE has been shut down, returns all fields to their default state. + +*******************************************************************************/ + +#include +#include +#include + +// Note: This function preserves a few parts of the fields - this means that if +// you are using this to reset a fresh FILE object, you should memset it +// to zeroes first to avoid problems. +extern "C" void fresetfile(FILE* fp) +{ + FILE* prev = fp->prev; + FILE* next = fp->next; + void* free_user = fp->free_user; + void (*free_func)(void*, FILE*) = fp->free_func; + int kept_flags = fp->flags & (_FILE_REGISTERED | 0); + memset(fp, 0, sizeof(*fp)); + fp->flags = kept_flags | _FILE_AUTO_LOCK; + fp->buffer_mode = -1; + fp->free_user = free_user; + fp->free_func = free_func; + fp->prev = prev; + fp->next = next; +} diff --git a/libc/fshutdown.cpp b/libc/fshutdown.cpp new file mode 100644 index 00000000..b87b5214 --- /dev/null +++ b/libc/fshutdown.cpp @@ -0,0 +1,45 @@ +/******************************************************************************* + + Copyright(C) Jonas 'Sortie' Termansen 2013. + + This file is part of the Sortix C Library. + + The Sortix C Library is free software: you can redistribute it and/or modify + it under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation, either version 3 of the License, or (at your + option) any later version. + + The Sortix C Library is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public + License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with the Sortix C Library. If not, see . + + fshutdown.cpp + Uninstalls the backend from a FILE so another can be reinstalled. + +*******************************************************************************/ + +#include +#include + +extern "C" int fshutdown(FILE* fp) +{ + int ret = fflush(fp); + if ( ret ) + { + /* TODO: How to report errors here? fclose may need us to return its + exact error value, for instance, as with popen/pclose. */; + } + ret = fp->close_func ? fp->close_func(fp->user) : ret; + if ( fp->flags & _FILE_BUFFER_OWNED ) + free(fp->buffer); + // Resetting the FILE here isn't needed in the case where fclose calls us, + // but it's nice to zero it out anyway (avoiding state) data, and it's a + // feature when called by freopen that wishes to reuse the FILE. It also + // means that the file is always in a consistent state. + fresetfile(fp); + return ret; +} diff --git a/libc/include/stdio.h b/libc/include/stdio.h index c8a0cb66..871586d4 100644 --- a/libc/include/stdio.h +++ b/libc/include/stdio.h @@ -164,12 +164,15 @@ extern char* tempnam(const char* dir, const char* pfx); #define fsetlocking __fsetlocking int fflush_stop_reading(FILE* fp); int fflush_stop_writing(FILE* fp); +void fdeletefile(FILE* fp); void fseterr(FILE* fp); void fregister(FILE* fp); +void fresetfile(FILE* fp); void funregister(FILE* fp); FILE* fnewfile(void); int fsetdefaultbuf(FILE* fp); int fcloseall(void); +int fshutdown(FILE* fp); int fpipe(FILE* pipes[2]); /* Internally used by standard library. */ #if defined(LIBC_LIBRARY) diff --git a/libc/stdio.c b/libc/stdio.c index f5e5a2e4..59d55dc7 100644 --- a/libc/stdio.c +++ b/libc/stdio.c @@ -35,9 +35,12 @@ FILE* stderr; int init_stdio() { - stdin = fdio_newfile(0, "r"); - stdout = fdio_newfile(1, "w"); - stderr = fdio_newfile(2, "w"); + // TODO: These calls require memory allocation and can fail - which we don't + // currently handle. How about declaring these as global objects and + // using fdio_install_fd instead? + stdin = fdio_new_fd(0, "r"); + stdout = fdio_new_fd(1, "w"); + stderr = fdio_new_fd(2, "w"); setvbuf(stderr, NULL, _IONBF, 0); return 0; }