[PATCH v4] ungetc: Guarantee single char pushback
Siddhesh Poyarekar
siddhesh@sourceware.org
Tue Dec 10 12:30:04 GMT 2024
The C standard requires that ungetc guarantees at least one pushback, so
put a single byte pushback buffer in the FILE struct to enable that.
Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
---
Changes from v3:
- Shrunk _flags2 and moved _short_backupbuf to the old FILE struct.
Changes from v2:
- Fixed nits
- Used _IO_free_backup_buf in oldfileops and wfileops as well.
- Enhanced test to try some more cases
Changes from v1:
- Drop ungetwc from scope of the patchset
- Fixed nits
- Retain old behaviour for legacy applications
- Minimize changes to fileops
- Namespace-ize free_backup_buf
- Add a test to verify that the subsequent malloc failure results in ungetc
failure too
- Add GNU Toolchain Authors copyright notice.
libio/bits/types/struct_FILE.h | 4 +-
libio/fileops.c | 7 +-
libio/genops.c | 23 +++++--
libio/libioP.h | 13 ++--
libio/oldfileops.c | 5 +-
libio/wfileops.c | 3 +-
stdio-common/Makefile | 2 +
stdio-common/tst-ungetc-nomem.c | 115 ++++++++++++++++++++++++++++++++
8 files changed, 154 insertions(+), 18 deletions(-)
create mode 100644 stdio-common/tst-ungetc-nomem.c
diff --git a/libio/bits/types/struct_FILE.h b/libio/bits/types/struct_FILE.h
index d8d26639d1..a5e0679de3 100644
--- a/libio/bits/types/struct_FILE.h
+++ b/libio/bits/types/struct_FILE.h
@@ -1,4 +1,5 @@
/* Copyright (C) 1991-2024 Free Software Foundation, Inc.
+ Copyright The GNU Toolchain Authors.
This file is part of the GNU C Library.
The GNU C Library is free software; you can redistribute it and/or
@@ -70,7 +71,8 @@ struct _IO_FILE
struct _IO_FILE *_chain;
int _fileno;
- int _flags2;
+ int _flags2:24;
+ char _short_backupbuf[1];
__off_t _old_offset; /* This used to be _offset but it's too small. */
/* 1+column number of pbase(); 0 is unknown. */
diff --git a/libio/fileops.c b/libio/fileops.c
index 759d737ec7..d49e489f55 100644
--- a/libio/fileops.c
+++ b/libio/fileops.c
@@ -1,4 +1,5 @@
/* Copyright (C) 1993-2024 Free Software Foundation, Inc.
+ Copyright The GNU Toolchain Authors.
This file is part of the GNU C Library.
The GNU C Library is free software; you can redistribute it and/or
@@ -480,7 +481,7 @@ _IO_new_file_underflow (FILE *fp)
/* Maybe we already have a push back pointer. */
if (fp->_IO_save_base != NULL)
{
- free (fp->_IO_save_base);
+ _IO_free_backup_buf (fp, fp->_IO_save_base);
fp->_flags &= ~_IO_IN_BACKUP;
}
_IO_doallocbuf (fp);
@@ -932,7 +933,7 @@ _IO_new_file_seekoff (FILE *fp, off64_t offset, int dir, int mode)
/* It could be that we already have a pushback buffer. */
if (fp->_IO_read_base != NULL)
{
- free (fp->_IO_read_base);
+ _IO_free_backup_buf (fp, fp->_IO_read_base);
fp->_flags &= ~_IO_IN_BACKUP;
}
_IO_doallocbuf (fp);
@@ -1282,7 +1283,7 @@ _IO_file_xsgetn (FILE *fp, void *data, size_t n)
/* Maybe we already have a push back pointer. */
if (fp->_IO_save_base != NULL)
{
- free (fp->_IO_save_base);
+ _IO_free_backup_buf (fp, fp->_IO_save_base);
fp->_flags &= ~_IO_IN_BACKUP;
}
_IO_doallocbuf (fp);
diff --git a/libio/genops.c b/libio/genops.c
index d7e35e67d5..dddd420ee2 100644
--- a/libio/genops.c
+++ b/libio/genops.c
@@ -1,4 +1,5 @@
/* Copyright (C) 1993-2024 Free Software Foundation, Inc.
+ Copyright The GNU Toolchain Authors.
This file is part of the GNU C Library.
The GNU C Library is free software; you can redistribute it and/or
@@ -48,6 +49,13 @@ flush_cleanup (void *not_used)
}
#endif
+void
+_IO_free_backup_buf (FILE *fp, char *ptr)
+{
+ if (ptr != fp->_short_backupbuf)
+ free (ptr);
+}
+
/* Fields in struct _IO_FILE after the _lock field are internal to
glibc and opaque to applications. We can change them as long as
the size of struct _IO_FILE is unchanged, which is checked as the
@@ -212,7 +220,7 @@ _IO_free_backup_area (FILE *fp)
{
if (_IO_in_backup (fp))
_IO_switch_to_main_get_area (fp); /* Just in case. */
- free (fp->_IO_save_base);
+ _IO_free_backup_buf (fp, fp->_IO_save_base);
fp->_IO_save_base = NULL;
fp->_IO_save_end = NULL;
fp->_IO_backup_base = NULL;
@@ -260,7 +268,7 @@ save_for_backup (FILE *fp, char *end_p)
memcpy (new_buffer + avail,
fp->_IO_read_base + least_mark,
needed_size);
- free (fp->_IO_save_base);
+ _IO_free_backup_buf (fp, fp->_IO_save_base);
fp->_IO_save_base = new_buffer;
fp->_IO_save_end = new_buffer + avail + needed_size;
}
@@ -636,7 +644,7 @@ _IO_default_finish (FILE *fp, int dummy)
if (fp->_IO_save_base)
{
- free (fp->_IO_save_base);
+ _IO_free_backup_buf (fp, fp->_IO_save_base);
fp->_IO_save_base = NULL;
}
@@ -998,11 +1006,14 @@ _IO_default_pbackfail (FILE *fp, int c)
else if (!_IO_have_backup (fp))
{
/* No backup buffer: allocate one. */
- /* Use nshort buffer, if unused? (probably not) FIXME */
int backup_size = 128;
char *bbuf = (char *) malloc (backup_size);
if (bbuf == NULL)
- return EOF;
+ {
+ /* Guarantee a 1-char pushback. */
+ bbuf = fp->_short_backupbuf;
+ backup_size = 1;
+ }
fp->_IO_save_base = bbuf;
fp->_IO_save_end = fp->_IO_save_base + backup_size;
fp->_IO_backup_base = fp->_IO_save_end;
@@ -1022,7 +1033,7 @@ _IO_default_pbackfail (FILE *fp, int c)
return EOF;
memcpy (new_buf + (new_size - old_size), fp->_IO_read_base,
old_size);
- free (fp->_IO_read_base);
+ _IO_free_backup_buf (fp, fp->_IO_read_base);
_IO_setg (fp, new_buf, new_buf + (new_size - old_size),
new_buf + new_size);
fp->_IO_backup_base = fp->_IO_read_ptr;
diff --git a/libio/libioP.h b/libio/libioP.h
index 34bf91fcd8..287caf8664 100644
--- a/libio/libioP.h
+++ b/libio/libioP.h
@@ -1,4 +1,5 @@
/* Copyright (C) 1993-2024 Free Software Foundation, Inc.
+ Copyright The GNU Toolchain Authors.
This file is part of the GNU C Library.
The GNU C Library is free software; you can redistribute it and/or
@@ -357,6 +358,8 @@ typedef FILE *_IO_ITER;
/* Generic functions */
+extern void _IO_free_backup_buf (FILE *, char *);
+libc_hidden_proto (_IO_free_backup_buf)
extern void _IO_switch_to_main_get_area (FILE *) __THROW;
extern void _IO_switch_to_backup_area (FILE *) __THROW;
extern int _IO_switch_to_get_mode (FILE *);
@@ -911,13 +914,13 @@ extern int _IO_vscanf (const char *, va_list) __THROW;
# define FILEBUF_LITERAL(CHAIN, FLAGS, FD, WDP) \
{ _IO_MAGIC+_IO_LINKED+_IO_IS_FILEBUF+FLAGS, \
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, (FILE *) CHAIN, FD, \
- 0, _IO_pos_BAD, 0, 0, { 0 }, &_IO_stdfile_##FD##_lock }
+ 0, { 0 }, _IO_pos_BAD, 0, 0, { 0 }, &_IO_stdfile_##FD##_lock }
# else
# define FILEBUF_LITERAL(CHAIN, FLAGS, FD, WDP) \
{ _IO_MAGIC+_IO_LINKED+_IO_IS_FILEBUF+FLAGS, \
NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, \
- NULL, NULL, (FILE *) CHAIN, FD, \
- 0, _IO_pos_BAD, 0, 0, { 0 }, &_IO_stdfile_##FD##_lock, _IO_pos_BAD,\
+ NULL, NULL, (FILE *) CHAIN, FD, 0, { 0 }, \
+ _IO_pos_BAD, 0, 0, { 0 }, &_IO_stdfile_##FD##_lock, _IO_pos_BAD, \
NULL, WDP, NULL }
# endif
#else
@@ -925,12 +928,12 @@ extern int _IO_vscanf (const char *, va_list) __THROW;
# define FILEBUF_LITERAL(CHAIN, FLAGS, FD, WDP) \
{ _IO_MAGIC+_IO_LINKED+_IO_IS_FILEBUF+FLAGS, \
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, (FILE *) CHAIN, FD, \
- 0, _IO_pos_BAD }
+ 0, { 0 }, _IO_pos_BAD }
# else
# define FILEBUF_LITERAL(CHAIN, FLAGS, FD, WDP) \
{ _IO_MAGIC+_IO_LINKED+_IO_IS_FILEBUF+FLAGS, \
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, (FILE *) CHAIN, FD, \
- 0, _IO_pos_BAD, 0, 0, { 0 }, 0, _IO_pos_BAD, \
+ 0, { 0 }, _IO_pos_BAD, 0, 0, { 0 }, 0, _IO_pos_BAD, \
NULL, WDP, 0 }
# endif
#endif
diff --git a/libio/oldfileops.c b/libio/oldfileops.c
index 8f775c9094..03f4d76a57 100644
--- a/libio/oldfileops.c
+++ b/libio/oldfileops.c
@@ -1,4 +1,5 @@
/* Copyright (C) 1993-2024 Free Software Foundation, Inc.
+ Copyright The GNU Toolchain Authors.
This file is part of the GNU C Library.
The GNU C Library is free software; you can redistribute it and/or
@@ -311,7 +312,7 @@ _IO_old_file_underflow (FILE *fp)
/* Maybe we already have a push back pointer. */
if (fp->_IO_save_base != NULL)
{
- free (fp->_IO_save_base);
+ _IO_free_backup_buf (fp, fp->_IO_save_base);
fp->_flags &= ~_IO_IN_BACKUP;
}
_IO_doallocbuf (fp);
@@ -464,7 +465,7 @@ _IO_old_file_seekoff (FILE *fp, off64_t offset, int dir, int mode)
/* It could be that we already have a pushback buffer. */
if (fp->_IO_read_base != NULL)
{
- free (fp->_IO_read_base);
+ _IO_free_backup_buf (fp, fp->_IO_read_base);
fp->_flags &= ~_IO_IN_BACKUP;
}
_IO_doallocbuf (fp);
diff --git a/libio/wfileops.c b/libio/wfileops.c
index 16beab1f3a..a96bfa589b 100644
--- a/libio/wfileops.c
+++ b/libio/wfileops.c
@@ -1,4 +1,5 @@
/* Copyright (C) 1993-2024 Free Software Foundation, Inc.
+ Copyright The GNU Toolchain Authors.
This file is part of the GNU C Library.
The GNU C Library is free software; you can redistribute it and/or
@@ -175,7 +176,7 @@ _IO_wfile_underflow (FILE *fp)
/* Maybe we already have a push back pointer. */
if (fp->_IO_save_base != NULL)
{
- free (fp->_IO_save_base);
+ _IO_free_backup_buf (fp, fp->_IO_save_base);
fp->_flags &= ~_IO_IN_BACKUP;
}
_IO_doallocbuf (fp);
diff --git a/stdio-common/Makefile b/stdio-common/Makefile
index e76e40e587..b1a04fd064 100644
--- a/stdio-common/Makefile
+++ b/stdio-common/Makefile
@@ -1,4 +1,5 @@
# Copyright (C) 1991-2024 Free Software Foundation, Inc.
+# Copyright The GNU Toolchain Authors.
# This file is part of the GNU C Library.
# The GNU C Library is free software; you can redistribute it and/or
@@ -303,6 +304,7 @@ tests := \
tst-tmpnam \
tst-ungetc \
tst-ungetc-leak \
+ tst-ungetc-nomem \
tst-unlockedio \
tst-vfprintf-mbs-prec \
tst-vfprintf-user-type \
diff --git a/stdio-common/tst-ungetc-nomem.c b/stdio-common/tst-ungetc-nomem.c
new file mode 100644
index 0000000000..49db33fff3
--- /dev/null
+++ b/stdio-common/tst-ungetc-nomem.c
@@ -0,0 +1,115 @@
+/* Test ungetc behavior with malloc failures.
+ Copyright The GNU Toolchain Authors.
+ This file is part of the GNU C Library.
+
+ The GNU 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 2.1 of the License, or (at your option) any later version.
+
+ The GNU 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 GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <stdio.h>
+#include <string.h>
+#include <support/check.h>
+#include <support/support.h>
+#include <support/temp_file.h>
+#include <support/xstdio.h>
+
+extern void *__libc_malloc (size_t)
+ __attribute__ ((malloc)) __attribute__ ((alloc_size (1)));
+
+static volatile bool fail = false;
+
+void *
+malloc (size_t sz)
+{
+ if (fail)
+ return NULL;
+
+ return __libc_malloc (sz);
+}
+
+static int
+do_test (void)
+{
+ char *filename = NULL;
+ struct stat props = {};
+ size_t bufsz = 0;
+
+ create_temp_file ("tst-ungetc-nomem.", &filename);
+ if (stat (filename, &props) != 0)
+ FAIL_EXIT1 ("Could not get file status: %m\n");
+
+ FILE *fp = fopen (filename, "w");
+
+ /* The libio buffer sizes are the same as block size. */
+ bufsz = props.st_blksize + 2;
+
+ char *buf = xmalloc (bufsz);
+ memset (buf, 'a', bufsz);
+
+ if (fwrite (buf, sizeof (char), bufsz, fp) != bufsz)
+ FAIL_EXIT1 ("fwrite failed: %m\n");
+ xfclose (fp);
+
+ /* Begin test. */
+ fp = xfopen (filename, "r");
+
+ while (!feof (fp))
+ {
+ /* Reset the pushback buffer state. */
+ fseek (fp, 0, SEEK_CUR);
+
+ fail = true;
+ /* 1: First ungetc should always succeed, as the standard requires. */
+ TEST_COMPARE (ungetc ('y', fp), 'y');
+
+ /* 2: This will result in resizing, which should fail. */
+ TEST_COMPARE (ungetc ('w', fp), EOF);
+
+ /* 3: Now allow the resizing, which should immediately fill up the buffer
+ too, since this allocates only double the current buffer, i.e.
+ 2-bytes. */
+ fail = false;
+ TEST_COMPARE (ungetc ('x', fp), 'x');
+
+ /* 4: And fail again because this again forces an alloc, which fails. */
+ fail = true;
+ TEST_COMPARE (ungetc ('x', fp), EOF);
+
+ /* 5: Enable allocations again so that we now get a 4-byte buffer. Now
+ both calls should work. */
+ fail = false;
+ TEST_COMPARE (ungetc ('x', fp), 'x');
+ fail = true;
+ TEST_COMPARE (ungetc ('x', fp), 'x');
+
+ /* Drain out the x's. */
+ TEST_COMPARE (fgetc (fp), 'x');
+ TEST_COMPARE (fgetc (fp), 'x');
+ TEST_COMPARE (fgetc (fp), 'x');
+
+ /* Finally, drain out the first char we had pushed back, followed by one more char
+ from the stream, if present. */
+ TEST_COMPARE (fgetc (fp), 'y');
+ char c = fgetc (fp);
+ if (!feof (fp))
+ TEST_COMPARE (c, 'a');
+ }
+
+ /* Final sanity check before we're done. */
+ TEST_COMPARE (ferror (fp), 0);
+ xfclose (fp);
+
+ return 0;
+}
+
+#include <support/test-driver.c>
--
2.47.1
More information about the Libc-alpha
mailing list