Summary: | fprintf() + fmemopen() error (?) | ||
---|---|---|---|
Product: | glibc | Reporter: | Michael Kerrisk <michael.kerrisk> |
Component: | libc | Assignee: | Ulrich Drepper <drepper.fsp> |
Status: | RESOLVED INVALID | ||
Severity: | normal | CC: | eblake, glibc-bugs, mtk.manpages |
Priority: | P2 | Flags: | fweimer:
security-
|
Version: | 2.3.5 | ||
Target Milestone: | --- | ||
Host: | Target: | ||
Build: | Last reconfirmed: |
Description
Michael Kerrisk
2005-12-08 13:34:03 UTC
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. 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. 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? 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
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
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
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. 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
Yeah, my preference is due to less buffer copying. 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
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. 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
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 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. Thanks for the comment Ulrich! Does anything else need to be done on this bugzilla entry? |