Bug 12836

Summary: fmemopen(buf,size,"w+b") isn't binary
Product: glibc Reporter: Eric Blake <eblake>
Component: stdioAssignee: Ulrich Drepper <drepper.fsp>
Status: RESOLVED FIXED    
Severity: normal CC: adhemerval.zanella, lynneandallan, mtk.manpages
Priority: P2 Flags: fweimer: security-
Version: 2.13   
Target Milestone: ---   
Host: Target:
Build: Last reconfirmed:

Description Eric Blake 2011-06-02 15:11:51 UTC
The documented behavior of fmemopen behaving in binary mode if 'b' was present in mode does not actually happen if 'b' is not in mode[1].  For consistency with fopen and friends, 'wb+' and 'w+b' should have identical effect.
Comment 1 Eric Blake 2011-06-02 15:13:26 UTC
diff --git i/libio/fmemopen.c w/libio/fmemopen.c
index 75f9f7e..ca7c604 100644
--- i/libio/fmemopen.c
+++ w/libio/fmemopen.c
@@ -251,7 +251,7 @@ fmemopen (void *buf, size_t len, const char *mode)
   else
     c->pos = 0;

-  c->binmode = mode[0] != '\0' && mode[1] == 'b';
+  c->binmode = mode[0] && mode[1 + (mode[1] == '+')] == 'b';

   iof.read = fmemopen_read;
   iof.write = fmemopen_write;
Comment 2 Ulrich Drepper 2011-07-23 04:00:08 UTC
An(In reply to comment #0)
> The documented behavior of fmemopen behaving in binary mode if 'b' was present
> in mode does not actually happen if 'b' is not in mode[1].  For consistency
> with fopen and friends, 'wb+' and 'w+b' should have identical effect.

Who says that?  Any such change would mean that either we'd have to change the version of the fmemopen interface and prevent code compiled now to use fmemopen from running with older glibcs.  And for what?  Just so that you get "consistency"?  No good reason.
Comment 3 Eric Blake 2011-07-27 17:59:04 UTC
(In reply to comment #2)
> An(In reply to comment #0)
> > The documented behavior of fmemopen behaving in binary mode if 'b' was present
> > in mode does not actually happen if 'b' is not in mode[1].  For consistency
> > with fopen and friends, 'wb+' and 'w+b' should have identical effect.
> 
> Who says that?

The Austin Group:

http://austingroupbugs.net/view.php?id=456

At lines 28743-28750 [XSH fmemopen DESCRIPTION], undo the TC1 changes
(XSH/TC1/D2/0136), and replace the existing text (in Issue 7, the
6 rows from "r or rb" to "at the first null byte") with:

r or rb Open the stream for reading.
w or wb Open the stream for writing.
a or ab Append; open the stream for writing.
r+ or rb+ or r+b Open the stream for update (reading and writing).
w+ or wb+ or w+b Open the stream for update (reading and writing).
  Truncate the buffer contents.
a+ or ab+ or a+b Append; open the stream for update (reading and
  writing).

>  Any such change would mean that either we'd have to change the
> version of the fmemopen interface and prevent code compiled now to use fmemopen
> from running with older glibcs.

This one-liner patch is forwards compatible:

Old programs have four defined choices documented in 'man fmemopen' when mode starts with 'w':

fmemopen (,"w") - text mode
fmemopen (,"wb") - binary mode
fmemopen (,"w+") - text mode
fmemopen (,"wb+") - binary mode
all other strings starting with w have undefined behavior

New programs will have five defined choices:
fmemopen (,"w") - text mode
fmemopen (,"wb") - binary mode
fmemopen (,"w+") - text mode
fmemopen (,"wb+") - binary mode
fmemopen (,"w+b") - binary mode
all other strings starting with w have undefined behavior

Old programs should not have been relying on undefined behavior of "w+b", and this patch merely adds one new mode, without changing the behavior of any existing defined modes.

I don't know why you are complaining about this not being backwards compatible.  I agree that a program compiled against glibc 2.15 (assuming that is the glibc version where this patch is added) but run against glibc 2.14 would do the wrong thing for w+b if fmemopen is not versioned for this fix.  But when have we ever supported that type of back compatibility?  In general, when you compile against a newer glibc, there are no promises that you can run your program against older glibc.  Furthermore, did you even version bump fmemopen when you added support for 'b' in the first place in bug 6544 - that is, if I use "wb" compiled against glibc 2.9 but then run against glibc 2.8, will I get a link failure due to a missing versioned symbol, or just a silent change in behavior because I was tickling behavior that was undefined in 2.8?  Likewise, did you version bump fopen when you fixed a mis-parse of the mode string in bug 12685?

At the end of the day, this patch is just a one-liner fix to a mis-parse of the mode string, and not a version compatibility nightmare that you are trying to make it out to be.

>  And for what?  Just so that you get
> "consistency"?  No good reason.

It's more than just "consistency", it is so that new programs will be able to use the new requirements to be added in Issue 8 of POSIX.
Comment 4 Michael Kerrisk 2012-04-28 02:23:59 UTC
I've added the following to the fmemopen() man page, under BUGS:

===
To specify binary mode for
.BR fmemopen ()
the \(aqb\(aq must be the
.I second
character in
.IR mode .
Thus, for example, "wb+" has the desired effect, but "w+b" does not.
This is inconsistent with the corresponding case in
.\" FIXME http://sourceware.org/bugzilla/show_bug.cgi?id=12836
.BR fopen (3).
===
Comment 5 paxdiablo 2014-01-30 03:50:54 UTC
The POSIX standard calls for fmemopen() to accept all mode strings as allowed in fopen() but notes that use of "b" results in implementation-defined behaviour.

So, technically, I think an implementation _could_ treat w+b and wb+ as different if it so desired but it would have to document that.

Of course, it would probably be easier to fix the code to make them identical :-)
Comment 6 Eric Blake 2014-01-30 04:23:18 UTC
(In reply to paxdiablo from comment #5)
> The POSIX standard calls for fmemopen() to accept all mode strings as
> allowed in fopen() but notes that use of "b" results in
> implementation-defined behaviour.

POSIX has been amended to cater to glibc's behavior of having different behavior for 'b':
http://austingroupbugs.net/view.php?id=396

And in fact, consensus was that the next version of POSIX should require glibc's behavior of having saner behavior for 'b':

http://austingroupbugs.net/view.php?id=456

> 
> So, technically, I think an implementation _could_ treat w+b and wb+ as
> different if it so desired but it would have to document that.
> 
> Of course, it would probably be easier to fix the code to make them
> identical :-)

No, we WANT them to be different, per the adjustments to POSIX.
Comment 7 Eric Blake 2014-01-30 04:24:55 UTC
(In reply to Eric Blake from comment #6)

> > So, technically, I think an implementation _could_ treat w+b and wb+ as
> > different if it so desired but it would have to document that.
> > 
> > Of course, it would probably be easier to fix the code to make them
> > identical :-)
> 
> No, we WANT them to be different, per the adjustments to POSIX.

Sorry, speaking too soon. We want 'w' and 'wb' to be different; but we want 'w+b' and 'wb+' to be identical, as that is what the next version of POSIX will require.  Glibc has to be fixed before it will comply with what is written in POSIX bug 456.
Comment 8 Adhemerval Zanella 2015-07-08 15:15:31 UTC
Closing bug since it should be fixed by fdb7d390dd0d96e4a8239c46f3aa64598b90842b.