This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v4] [BZ #12836] Make fmemopen(buf,size,"w+b") binary.


> 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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]