[PATCH v2] libio: Fix fmemopen_write on appending condition

[Adhemerval Zanella Netto]

On 24/03/26 14:36, Rocket Ma wrote:
> Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> 于2026年3月24日周二 22:39写道：
>> This test is missing a Copyright and I think it should be placed on stdio-common/
>> subfolder.
> 
> Copyright added. I don't think it should be placed in stdio-common.
> Since this bug is directly from
> libio/fmemopen.c, place it in stdio-common would be confusing.
> 
>> We prefer to use support/check.h macros.
> 
> Done.
> 
>>> +
>>> +static int
>>> +do_test (void)
>>> +{
>>> +  char buf[5] = "1";
>>> +  FILE *fp = fmemopen (buf, 4, "a+");
>>> +  tst_assert (fp != NULL);
>>> +  tst_assert (fseek (fp, 3, SEEK_SET) == 0);
>>> +  tst_assert (fwrite ("XXXX", 1, 4, fp) > 0);
>>
>> This test does not really stress the issue, it works regardless of the patch
>> is applied or not.  We need also to check if the string does have the XXX append
>> on it:
>>
>> static int
>> do_test (void)
>> {
>>   char buf[5] = "1";
>>   FILE *fp = xfmemopen (buf, 4, "a+");
>>   TEST_COMPARE (fseek (fp, 3, SEEK_SET), 0);
>>   TEST_VERIFY (fwrite ("XXXX", 1, 4, fp) > 0);
>>   int r = fclose (fp);
>>   printf ("r=%d errno=%s\n", r, strerrorname_np (errno));
>>   TEST_COMPARE_STRING (buf, "1XXX");
>>
>>   return 0;
>> }
>>
> 
> I sebuf to NULL so fwrite could immediately write to underlying buf, then
> we can compare it with "1XXX" immediately.

I think setbuf is not strictly required here, with this fix fclose does write
on the input buffer. It also masks a potential issue, as below. I think we
need to test *all* possible buffer modes.

> 
>>> +
>>> +  return 0;
>>> +}
>>> +
>>> +#include <support/test-driver.c>
>>> diff --git a/libio/fmemopen.c b/libio/fmemopen.c
>>> index f2ae1338d3..cdc3a3476e 100644
>>> --- a/libio/fmemopen.c
>>> +++ b/libio/fmemopen.c
>>> @@ -71,7 +71,7 @@ fmemopen_write (void *cookie, const char *b, size_t s)
>>>
>>>    if (pos + s > c->size)
>>>      {
>>> -      if ((size_t) (c->pos + addnullc) >= c->size)
>>> +      if ((size_t) (pos + addnullc) >= c->size)
>>>       {
>>>         __set_errno (ENOSPC);
>>>         return 0;
>>
>> I think this does not fully fix the issue, since the fwrite below
>> will still return 4 where I would expect 3 bytes are written
>> (buf becomes "1XXX"), the null terminator attempt lands at size = 4
>> (boundary, not within the array).
> 
> Please see Bug 34006 (https://sourceware.org/bugzilla/show_bug.cgi?id=34006),
> before considering that, I think we need to discuss on a consistent behavior of
> fwrite.

Yes, and I agree with your rationale. The issue is, with this patch,
fclose in buffered mode (the default) now returns a failure *without* 
setting errno and this is inconsistent.

I am not sure if fmemopen_write should hide that less bytes are written
in the input buffer (thus running 's') or if we should fix it on the
fwrite (so it only advances its internal position 3 instead of 4 and
then issues fmemopen_write with s=3 instead of s4).

> 
>> Unfortunately, the current glibc implementation only calls
>> fmemopen_write at flush, and thus the fclose above will fail without
>> setting the errno. At least with current code we properly fail with
>> ENOSPC in such cases.
>>
>> I think we will need to proper fix the fwrite return code before,
>> so fflush/fclose does not fail in this case.
> 
> But the "buf" is buffered by stdio... Currently it should be able to
> fail when no one byte can
> be written in fflush; then for fclose, I'm not sure when closing a
> normal FILE, flushing rest buffer
> to underlying fd, what if write failed, for example, lack of space?
> Will fclose fail then? If so,
> we can consider handle this case.

I think we should have the fmemopen size information somewhere in the
FILE so _IO_file_doallocate can allocate a buffer with a size limit
instead of a BUFSIZ one.  One we can init the buffer size on __fmemopen,
but it pessimize memory allocation on FILE creation instead of when
data is actually done.  Something as below:

It has the size effect of making a fwrite that overflow the input
buffer to signal the numbers of bytes that will be written in a fflush
or fclose, instead of silent fail.  But it triggers a regression
on stdio-common/tst-fmemopen2 because it seems that we do expect this
for buffer size of 0 (not sure if this test is fully valid though).


diff --git a/libio/filedoalloc.c b/libio/filedoalloc.c
index 103035d398..07e09e9718 100644
--- a/libio/filedoalloc.c
+++ b/libio/filedoalloc.c
@@ -61,16 +61,12 @@
 #include <stdlib.h>
 #include <unistd.h>

-/* Allocate a file buffer, or switch to unbuffered I/O.  Streams for
-   TTY devices default to line buffered.  */
 int
-_IO_file_doallocate (FILE *fp)
+_IO_file_doallocate_size (FILE *fp, size_t size)
 {
-  size_t size;
   char *p;
   struct __stat64_t64 st;

-  size = BUFSIZ;
   if (fp->_fileno >= 0 && __builtin_expect (_IO_SYSSTAT (fp, &st), 0) >= 0)
     {
       if (S_ISCHR (st.st_mode))
@@ -94,4 +90,12 @@ _IO_file_doallocate (FILE *fp)
   _IO_setb (fp, p, p + size, 1);
   return 1;
 }
+
+/* Allocate a file buffer, or switch to unbuffered I/O.  Streams for
+   TTY devices default to line buffered.  */
+int
+_IO_file_doallocate (FILE *fp)
+{
+  return _IO_file_doallocate_size (fp, BUFSIZ);
+}
 libc_hidden_def (_IO_file_doallocate)
diff --git a/libio/fmemopen.c b/libio/fmemopen.c
index f2ae1338d3..b2e9541365 100644
--- a/libio/fmemopen.c
+++ b/libio/fmemopen.c
@@ -71,7 +71,7 @@ fmemopen_write (void *cookie, const char *b, size_t s)

   if (pos + s > c->size)
     {
-      if ((size_t) (c->pos + addnullc) >= c->size)
+      if ((size_t) (pos + addnullc) >= c->size)
        {
          __set_errno (ENOSPC);
          return 0;
@@ -219,6 +219,8 @@ __fmemopen (void *buf, size_t len, const char *mode)

       free (c);
     }
+  else
+    _IO_file_doallocate_size (result, len);

   return result;
 }
diff --git a/libio/libioP.h b/libio/libioP.h
index 1485d22619..7938a62c0b 100644
--- a/libio/libioP.h
+++ b/libio/libioP.h
@@ -596,6 +596,7 @@ extern void _IO_old_init (FILE *fp, int flags) __THROW;
 /* Jumptable functions for files. */

 extern int _IO_file_doallocate (FILE *) __THROW;
+extern int _IO_file_doallocate_size (FILE *, size_t) attribute_hidden;
 libc_hidden_proto (_IO_file_doallocate)
 extern FILE* _IO_file_setbuf (FILE *, char *, ssize_t);
 libc_hidden_proto (_IO_file_setbuf)