[Bug libc/1995] New: fprintf() + fmemopen() error (?)

[Bug libc/1995] New: fprintf() + fmemopen() error (?)

[michael dot kerrisk at gmx dot net]

When run, the program below displays the following output:

    $ ./a.out
    ret = 19 buf=123456789

However, since the fprintf() is causing an overflow in 'buf'
shouldn't the call be returning -1 as an error indication (as 
would happen when fprintf() fails because the disk is full)?

Cheers,

Michael

define _GNU_SOURCE
#include <stdio.h>

int main()
{
  char buf[10];
  FILE *fp;
  int ret;

  fp = fmemopen(buf, sizeof(buf), "w");
  ret = fprintf(fp, "%s", "1234567890123456789");
  fclose(fp);
  printf("ret = %d buf=%s\n", ret, buf);
  return 0;
}

-- 
           Summary: fprintf() + fmemopen() error (?)
           Product: glibc
           Version: 2.3.5
            Status: NEW
          Severity: normal
          Priority: P2
         Component: libc
        AssignedTo: drepper at redhat dot com
        ReportedBy: michael dot kerrisk at gmx dot net
                CC: glibc-bugs at sources dot redhat dot com


http://sourceware.org/bugzilla/show_bug.cgi?id=1995

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/1995] fprintf() + fmemopen() error (?)

[rsa at us dot ibm dot com]

------- Additional Comments From rsa at us dot ibm dot com  2006-03-31 18:56 -------
I checked through the C99 spec and "The Open Group Base Specifications Issue 6
IEEE Std 1003.1, 2004 Edition" and couldn't find any expressed behavior
regarding the return value of fprintf when only partial data is written.

investigation:

In this case fmemopen() sets the fp stream's internal write function to:
iof.write = fmemopen_write;

In libio/fmemopen.c
fmemopen_write() only writes 's' number of characters where 's' is the number of
characters that can fit in the remaining buffer space:

s = c->size - c->pos - addnullc;

In stdio-common/vfprintf.c we see that PUT should call on the 's' (fp) stream's
fopenmem_write function:

#define outstring(String, Len)                                                \
  do                                                                          \
    {                                                                         \
      if ((size_t) PUT (s, (String), (Len)) != (size_t) (Len))                \
        {                                                                     \
          done = -1;                                                          \
          goto all_done;                                                      \
        }                                                                     \
      done += (Len);                                                          \
    }                                                                         \
  while (0)

In the case where 'Len' exceeds the space in the buffer 'done' will be set to -1.

Since fopenmem_write() doesn't return 'Len', but only the amount of data written
to the buffer there seems to be a problem if '-1' isn't being returned for this
test case.

I intend to check whether fopenmem_write() really is being invoked when
'outstring' is called.



-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=1995

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/1995] fprintf() + fmemopen() error (?)

[rsa at us dot ibm dot com]

------- Additional Comments From rsa at us dot ibm dot com  2006-04-11 22:49 -------
Michael Kerrisk,

Unfortunately you've been bitten by GLIBC default stream buffering, which is
_IOFBF, being 'fully buffered' which is synonymous in the manual pages with
'block buffered'.

This means that the stream has an internal buffer and it holds data in that
buffer until such time that it needs to write it to a file (which is 'buf[10]').
 It generally writes to the file when prompted via a fflush() or an fclose().

In your source code example only when the file is closed is the fmemopen_write()
function invoked.  The buffer buf[10] isn't ever overwritten because
fmemopen_write() prevents that from happening.

The invocation of fprintf() against the stream always succeeds because the
stream's internal buffer is large enough to hold all 19 characters.

To get fprintf() to report in the manner you desire you can set the stream
buffering mode to _IONBF, 'unbuffered' using setvbuf().  This will direct
fprintf() to write directly to the file (buf[10]), which will invoke
fmemopen_write() directly.  You'll get the '-1' return value from fprintf() that
you're looking for.  Otherwise do your own accounting on avail space in buf
before you call fprintf.

Please reference the following source:

#define _GNU_SOURCE
#include <stdio.h>

int main()
{
  char buf[10];
  FILE *fp;
  int ret;

  fp = fmemopen(&buf[0], sizeof(buf), "w");
  setvbuf(fp, (char *)NULL, _IONBF, 0);

  /* write 19 and see it fail */
  ret = fprintf(fp, "%s", "1234567890123456789");
  printf("ret = %d buf=%s\n", ret, buf);

  /* reset the stream pointer to the beginning. */
  (void)fseek(fp, 0L, SEEK_SET);
  /* write 9 exactly and see it succeed */
  ret = fprintf(fp, "%s", "abcdefghi");
  printf("ret =  %d buf=%s\n", ret, buf);

  fclose(fp);
  return 0;
}

There is no bug in glibc.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |INVALID


http://sourceware.org/bugzilla/show_bug.cgi?id=1995

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/1995] fprintf() + fmemopen() error (?)

[rsa at us dot ibm dot com]

------- Additional Comments From rsa at us dot ibm dot com  2006-04-12 22:20 -------
My dismissal may have been a bit premature.

A possible fix may be to call _IO_setbuffer() on the _IO_FILE * returned from
the call to _IO_fopencookie(), just prior to fmemopen() function return.  My
reservation is that we may not want to implicitly change to a form of stream
buffering that isn't the expected system wide default.

Ulrich, would you care to comment? 

-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=1995

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/1995] fprintf() + fmemopen() error (?)

[michael dot kerrisk at gmx dot net]

------- Additional Comments From michael dot kerrisk at gmx dot net  2006-04-19 02:14 -------
Subject: Re:  fprintf() + fmemopen() error (?)

Hi Ryan,

Thanks for your reply.

> Unfortunately you've been bitten by GLIBC default stream buffering, which
> is _IOFBF, being 'fully buffered' which is synonymous in the manual pages
> with 'block buffered'.

Doh!  Got it now.  Easy mistake to make though.  Perhaps a word or 
two in the 'info fmemopen' page suggesting the use of 
setvbuf()/setbuf() would help?

Cheers,

Michael



-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=1995

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/1995] fprintf() + fmemopen() error (?)

[tschwinge at gnu dot org]

------- Additional Comments From tschwinge at gnu dot org  2006-04-19 12:17 -------
Subject: Re:  fprintf() + fmemopen() error (?)

On Wed, Apr 19, 2006 at 02:14:54AM -0000, michael dot kerrisk at gmx dot net wrote:
> > Unfortunately you've been bitten by GLIBC default stream buffering, which
> > is _IOFBF, being 'fully buffered' which is synonymous in the manual pages
> > with 'block buffered'.
> 
> Doh!  Got it now.  Easy mistake to make though.  Perhaps a word or 
> two in the 'info fmemopen' page suggesting the use of 
> setvbuf()/setbuf() would help?

Well, what about _you_ submitting a tiny patch to make the documentation
more useful for you?  :-)


Regards,
 Thomas


-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=1995

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/1995] fprintf() + fmemopen() error (?)

[michael dot kerrisk at gmx dot net]

------- Additional Comments From michael dot kerrisk at gmx dot net  2006-04-19 19:52 -------
Subject: Re:  fprintf() + fmemopen() error (?)

> 
> ------- Additional Comments From tschwinge at gnu dot org  2006-04-19 12:17
> ------- Subject: Re:  fprintf() + fmemopen() error (?)
> 
> On Wed, Apr 19, 2006 at 02:14:54AM -0000, michael dot kerrisk at gmx dot
> net wrote: > > Unfortunately you've been bitten by GLIBC default stream
> buffering, which > > is _IOFBF, being 'fully buffered' which is synonymous
> in the manual pages > > with 'block buffered'. > > Doh!  Got it now.  Easy
> mistake to make though.  Perhaps a word or > two in the 'info fmemopen'
> page suggesting the use of > setvbuf()/setbuf() would help?
> 
> Well, what about _you_ submitting a tiny patch to make the documentation
> more useful for you?  :-)

Sure.

Here's the text that man-pages has just acquired for fmemopen.3

[[
Attempts  to  write  more  than  size  bytes to the buffer result in an
error.  (Such errors will only be visible  when  the  stdio  buffer  is
flushed.   Disabling  buffering  with setbuf(fp, NULL) may be useful to
detect errors at the time of an output operation.)
]]

Cheers,

Michael


-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=1995

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/1995] fprintf() + fmemopen() error (?)

[rsa at us dot ibm dot com]

------- Additional Comments From rsa at us dot ibm dot com  2006-04-19 20:05 -------
Hi Michael,

In my tests I also found that setting stream buffering to the size of buf works
just as well as no-buffering and is probably a better method because it reduces
turnaround, albeit slightly, i.e. 

setbuffer(fp, buf, sizeof(buf));

I actually sent a patch to libc-alpha that got lost in the ether somewhere
(probably sitting in limbo on an ibm relay server again) that calls
_IO_setbuffer() on the _IO_FILE * stream pointer in fmemopen() after the
_IO_fopencookie() call in case it was deemed acceptable to change the buffering
implicitly.  I'll resend for comment tomorrow if the patch doesn't show up.

I suggest mentioning setbuffer(fp, buf, sizeof(buf)) in the man page as the
recommended method for getting around the problem.  I believe the libc info page
needs to be updated as well.

-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=1995

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/1995] fprintf() + fmemopen() error (?)

[michael dot kerrisk at gmx dot net]

------- Additional Comments From michael dot kerrisk at gmx dot net  2006-04-19 21:00 -------
Subject: Re:  fprintf() + fmemopen() error (?)

Hi Ryan,
 
> I suggest mentioning setbuffer(fp, buf, sizeof(buf)) in the man page as the
> recommended method for getting around the problem.  I believe the libc info
> page needs to be updated as well.

Can you just say a little more about why you think 

setbuffer(fp, buf, sizeof(buf)) 

is superior to, say, 

setvbuf(fp, NULL)

Yes, I suppose that there is potentially less buffer copying involved with 
the former way of doing things, but is there anything else?

For what it's worth, I like the idea of your patch that would make the 
setbuffer() step implicit.

Cheers,

Michael


-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=1995

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/1995] fprintf() + fmemopen() error (?)

[rsa at us dot ibm dot com]

------- Additional Comments From rsa at us dot ibm dot com  2006-04-19 21:41 -------
Yeah, my preference is due to less buffer copying.

-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=1995

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/1995] fprintf() + fmemopen() error (?)

[michael dot kerrisk at gmx dot net]

------- Additional Comments From michael dot kerrisk at gmx dot net  2006-04-19 23:24 -------
Subject: Re:  fprintf() + fmemopen() error (?)

> Yeah, my preference is due to less buffer copying.

Thanks Ryan.

For the moment, the man page now reads:

       Attempts to write more  than  size  bytes  to  the  buffer
       result in an error.  (By default, such errors will only be
       visible when  the  stdio  buffer  is  flushed.   Disabling
       buffering  with  setbuf(fp, NULL)  may be useful to detect
       errors at the time of an output operation.  Alternatively,
       the  caller  can  explicitly  set  buf as the stdio stream
       buffer, at the same time informing stdio of  the  buffer's
       size, using setbuffer(fp, buf, size).)

Cheers,

Michael


-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=1995

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/1995] fprintf() + fmemopen() error (?)

[rsa at us dot ibm dot com]

------- Additional Comments From rsa at us dot ibm dot com  2006-04-20 15:35 -------
Please reference libc-alpha post:

http://sources.redhat.com/ml/libc-alpha/2006-04/msg00063.html

for a proposed patch which fixes the problem in fmemopen() by implicitly setting
the stream buffer size to the size of the backing buffer.

-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=1995

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/1995] fprintf() + fmemopen() error (?)

[michael dot kerrisk at gmx dot net]

------- Additional Comments From michael dot kerrisk at gmx dot net  2006-04-20 19:33 -------
Subject: Re:  fprintf() + fmemopen() error (?)

> Please reference libc-alpha post:
> 
> http://sources.redhat.com/ml/libc-alpha/2006-04/msg00063.html
> 
> for a proposed patch which fixes the problem in fmemopen() by implicitly
> setting the stream buffer size to the size of the backing buffer.

Thanks.  I've put it on my "watchlist".

Cheers,

Michael


-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=1995

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/1995] fprintf() + fmemopen() error (?)

[rsa at us dot ibm dot com]

------- Additional Comments From rsa at us dot ibm dot com  2006-04-21 13:03 -------
Whoops, had a malformed change log on that patch.  Here's the new libc-alpha
post/patch to watch:

http://sources.redhat.com/ml/libc-alpha/2006-04/msg00064.html

-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=1995

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/1995] fprintf() + fmemopen() error (?)

[drepper at redhat dot com]

------- Additional Comments From drepper at redhat dot com  2006-04-23 17:03 -------
I do not intend to change the code.  The problem is that with this change of the
*default* behavior some programs might ill-function.  I don't say correct
programs.  But programs so far can recognize bugs by checking the fclose status
only.  By flagging the overflow at printf time this would change and those
programs might mysteriously fail.

Programs which want the early failures can easily use setvbuf etc on their own.
 No reason to potentially upset programs.

-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=1995

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/1995] fprintf() + fmemopen() error (?)

[rsa at us dot ibm dot com]

------- Additional Comments From rsa at us dot ibm dot com  2006-04-24 15:20 -------
Thanks for the comment Ulrich!

Does anything else need to be done on this bugzilla entry?

-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=1995

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.