From 89eb6c329b49af2a01c15e51611d992c717ddabc Mon Sep 17 00:00:00 2001 From: HanaMAshour <167436481+HanaMAshour@users.noreply.github.com> Date: Tue, 23 Apr 2024 15:09:15 +0000 Subject: [PATCH 1/4] tinystdio: ungetc decrements file position indicator correctly According to the ISO/IEC_9899_1999, section: J.5.16 it is mentioned that there is a file position indicator that is decremented by each call of the ungetc function. The value of the file position indicator was not directed to the right position, this issue is now handled in the ftell function so the file indicator is placed correctly after the function ungetc is called. An "if" condition is added which checks on the value of unget and if it is not equal zero then ungetc function is called so the indicator will be decremented. Signed-off-by: Hana Ashour --- newlib/libc/tinystdio/ftell.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/newlib/libc/tinystdio/ftell.c b/newlib/libc/tinystdio/ftell.c index b467fa8be2..4dd8c5d122 100644 --- a/newlib/libc/tinystdio/ftell.c +++ b/newlib/libc/tinystdio/ftell.c @@ -43,8 +43,14 @@ FSEEK_TYPE ftell(FILE *stream) { struct __file_ext *xf = (struct __file_ext *) stream; - if ((stream->flags & __SEXT) && xf->seek) - return (FSEEK_TYPE) (xf->seek) (stream, 0, SEEK_CUR); + FSEEK_TYPE ret; + if ((stream->flags & __SEXT) && xf->seek) { + ret = (FSEEK_TYPE) (xf->seek) (stream, 0, SEEK_CUR); + if(stream->unget != 0) + return --ret; + else + return ret; + } errno = ESPIPE; return -1; } From f5140fe444f4eca8ecf351e4eefb40abb637a7e6 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Wed, 24 Apr 2024 12:42:59 -0700 Subject: [PATCH 2/4] tinystdio: Add __atomic_load_ungetc This allows fetching the unget value using stdatomic interfaces to ensure atomicity wrt other unget operations. Signed-off-by: Keith Packard --- newlib/libc/tinystdio/atomic_load.c | 48 +++++++++++++++++++++++++++ newlib/libc/tinystdio/meson.build | 1 + newlib/libc/tinystdio/stdio_private.h | 17 ++++++++++ 3 files changed, 66 insertions(+) create mode 100644 newlib/libc/tinystdio/atomic_load.c diff --git a/newlib/libc/tinystdio/atomic_load.c b/newlib/libc/tinystdio/atomic_load.c new file mode 100644 index 0000000000..0341d476a0 --- /dev/null +++ b/newlib/libc/tinystdio/atomic_load.c @@ -0,0 +1,48 @@ +/* + * SPDX-License-Identifier: BSD-3-Clause + * + * Copyright © 2024 Keith Packard + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials provided + * with the distribution. + * + * 3. Neither the name of the copyright holder nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS + * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE + * COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR + * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED + * OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#include "stdio_private.h" + +#if defined(ATOMIC_UNGETC) && !defined(PICOLIBC_HAVE_SYNC_COMPARE_AND_SWAP) + +__ungetc_t +__picolibc_non_atomic_load_ungetc(const volatile __ungetc_t *p) +{ + return __non_atomic_load_ungetc(p); +} + +__weak_reference(__picolibc_non_atomic_load_ungetc, __atomic_load_ungetc); + +#endif diff --git a/newlib/libc/tinystdio/meson.build b/newlib/libc/tinystdio/meson.build index e52ac5a5e5..b0161519e8 100644 --- a/newlib/libc/tinystdio/meson.build +++ b/newlib/libc/tinystdio/meson.build @@ -34,6 +34,7 @@ # srcs_tinystdio = [ 'asprintf.c', + 'atomic_load.c', 'atold_engine.c', 'bufio.c', 'clearerr.c', diff --git a/newlib/libc/tinystdio/stdio_private.h b/newlib/libc/tinystdio/stdio_private.h index bdc814d287..ea4748de14 100644 --- a/newlib/libc/tinystdio/stdio_private.h +++ b/newlib/libc/tinystdio/stdio_private.h @@ -580,6 +580,12 @@ __non_atomic_compare_exchange_ungetc(__ungetc_t *p, __ungetc_t d, __ungetc_t v) return true; } +static inline uint16_t +__non_atomic_load_ungetc(const volatile __ungetc_t *p) +{ + return *p; +} + #ifdef ATOMIC_UNGETC #if __PICOLIBC_UNGETC_SIZE == 4 && defined (__GCC_HAVE_SYNC_COMPARE_AND_SWAP_4) @@ -608,6 +614,12 @@ __atomic_exchange_ungetc(__ungetc_t *p, __ungetc_t v) return atomic_exchange_explicit(pa, v, memory_order_relaxed); } +static inline __ungetc_t +__atomic_load_ungetc(const volatile __ungetc_t *p) +{ + _Atomic __ungetc_t *pa = (_Atomic __ungetc_t *) p; + return atomic_load(pa); +} #else bool @@ -616,6 +628,9 @@ __atomic_compare_exchange_ungetc(__ungetc_t *p, __ungetc_t d, __ungetc_t v); __ungetc_t __atomic_exchange_ungetc(__ungetc_t *p, __ungetc_t v); +__ungetc_t +__atomic_load_ungetc(const volatile __ungetc_t *p); + #endif /* PICOLIBC_HAVE_SYNC_COMPARE_AND_SWAP */ #else @@ -624,6 +639,8 @@ __atomic_exchange_ungetc(__ungetc_t *p, __ungetc_t v); #define __atomic_exchange_ungetc(p,v) __non_atomic_exchange_ungetc(p,v) +#define __atomic_load_ungetc(p) (*(p)) + #endif /* ATOMIC_UNGETC */ /* From 5badc7ba4c2c161ce17a2bd6787a57060112138c Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Wed, 24 Apr 2024 12:45:31 -0700 Subject: [PATCH 3/4] tinystdio: Check for pending unget in ftell A pending unget value means that the underlying reported seek position will be off by one. Signed-off-by: Keith Packard --- newlib/libc/tinystdio/ftell.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/newlib/libc/tinystdio/ftell.c b/newlib/libc/tinystdio/ftell.c index b467fa8be2..c2cd0f5e8a 100644 --- a/newlib/libc/tinystdio/ftell.c +++ b/newlib/libc/tinystdio/ftell.c @@ -43,8 +43,12 @@ FSEEK_TYPE ftell(FILE *stream) { struct __file_ext *xf = (struct __file_ext *) stream; - if ((stream->flags & __SEXT) && xf->seek) - return (FSEEK_TYPE) (xf->seek) (stream, 0, SEEK_CUR); + if ((stream->flags & __SEXT) && xf->seek) { + FSEEK_TYPE ret = (FSEEK_TYPE) (xf->seek) (stream, 0, SEEK_CUR); + if (__atomic_load_ungetc(&stream->unget) != 0) + ret--; + return ret; + } errno = ESPIPE; return -1; } From 8272f299f6124e2fc426b7db08d74c2a62ca1434 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Wed, 24 Apr 2024 12:47:10 -0700 Subject: [PATCH 4/4] test: Test ftell in combination with ungetc Make sure ungetc affects ftell value correctly. Signed-off-by: Keith Packard --- test/meson.build | 2 +- test/test-ungetc-ftell.c | 75 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 1 deletion(-) create mode 100644 test/test-ungetc-ftell.c diff --git a/test/meson.build b/test/meson.build index f897a7d349..91c565bf61 100644 --- a/test/meson.build +++ b/test/meson.build @@ -533,7 +533,7 @@ foreach target : targets # legacy stdio doesn't work on semihosting, so just skip it if tinystdio - plain_tests += ['test-fopen', 'test-mktemp', 'test-fread-fwrite'] + plain_tests += ['test-fopen', 'test-mktemp', 'test-fread-fwrite', 'test-ungetc-ftell'] endif endif diff --git a/test/test-ungetc-ftell.c b/test/test-ungetc-ftell.c new file mode 100644 index 0000000000..245679814f --- /dev/null +++ b/test/test-ungetc-ftell.c @@ -0,0 +1,75 @@ +/* + * SPDX-License-Identifier: BSD-3-Clause + * + * Copyright © 2024 Hana Ashour + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials provided + * with the distribution. + * + * 3. Neither the name of the copyright holder nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS + * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE + * COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR + * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED + * OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#include +#include + +const char *str = "This is a string"; + +int main(void) { + + FILE *file; + char first; + long position, start; + + file = fopen( "testfile.dat", "wb" ); + if( file == NULL ) return 1; + fputs(str, file); + fclose(file); + + file = fopen( "testfile.dat", "rb" ); + if(file == NULL) return 1; + + first = fgetc(file); + printf("First character read: %c\n", first); + + start = ftell(file); + printf("Position before ungetc: %ld\n", start); + + ungetc(first, file); // Use ungetc to put the character back + + position = ftell(file); // Check ftell position (should be 0 after ungetc if first character was read) + printf("Position after ungetc: %ld\n", position); + + fclose(file); + + if (position == 0) { + printf("Test passed: ungetc and ftell working as expected.\n"); + return 0; + } else { + printf("Test failed: Incorrect position after ungetc.\n"); + return 1; + } +}