This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v4] [BZ #12836] Make fmemopen(buf,size,"w+b") binary.
- From: Roland McGrath <roland at hack dot frob dot com>
- To: OndÅej BÃlka <neleai at seznam dot cz>
- Cc: "Joseph S. Myers" <joseph at codesourcery dot com>, libc-alpha at sourceware dot org
- Date: Wed, 29 May 2013 15:32:49 -0700 (PDT)
- Subject: Re: [PATCH v4] [BZ #12836] Make fmemopen(buf,size,"w+b") binary.
- References: <20130509161648 dot GA31713 at domone dot kolej dot mff dot cuni dot cz> <Pine dot LNX dot 4 dot 64 dot 1305091636200 dot 25142 at digraph dot polyomino dot org dot uk> <20130514115947 dot GA7284 at domone dot kolej dot mff dot cuni dot cz> <20130520082527 dot GB32195 at domone dot kolej dot mff dot cuni dot cz> <20130523190913 dot 04DBA2C09E at topped-with-meat dot com> <20130524091603 dot GA10400 at domone dot kolej dot mff dot cuni dot cz>
> Added comments except
>
> On Thu, May 23, 2013 at 12:09:12PM -0700, Roland McGrath wrote:
> > > + for (i = 0 ; i < strlen (s) ; i++)
> >
> > Use sizeof. No spaces before ;.
> Here strlen is rigth in case that in future tests s contains 0.
What "future tests"? This is the program. It does what it does.
If later you change it to do something different, then change it.
> > > + printf ("Not opened in binary mode.\n");
> >
> > Use puts.
> Why?
printf is for formatted output. You are doing constant output.
Write what you mean. (Also it's shorter.)
> * libio/fmemopen.c (fmemopen): Make mode "w+b" binary.
This gives an example and makes it sound like it's a special case or
something. I'd say, "Notice 'b' suffix anywhere in MODE, not just in
MODE[1]."
> * libio/Makefile (tests): Add it.
> * libio/test-fmemopen-bz12836.c: New file.
The "Add it" makes sense only when that line follows the line that defines
what "it" means, not when it precedes it.
Again, a name like "bug-fmemopen-openmode.c" would be better.
> - c->binmode = mode[0] != '\0' && mode[1] == 'b';
> + c->binmode = 0;
> + for (int i = 0; mode[i] != '\0'; i++)
> + if (mode[i] == 'b')
> + c->binmode = 1;
c->binmode = strchr (mode, 'b') != NULL;
Thanks,
Roland
- References:
- [PATCH] [BZ #12836] Make fmemopen(buf,size,"w+b") binary.
- Re: [PATCH] [BZ #12836] Make fmemopen(buf,size,"w+b") binary.
- [PATCH v2] [BZ #12836] Make fmemopen(buf,size,"w+b") binary.
- Re: [PATCH v3] [BZ #12836] Make fmemopen(buf,size,"w+b") binary.
- Re: [PATCH v3] [BZ #12836] Make fmemopen(buf,size,"w+b") binary.
- Re: [PATCH v4] [BZ #12836] Make fmemopen(buf,size,"w+b") binary.