Re: [PATCH v2][BZ #13152] fmemopen does not honor append mode.

[Roland McGrath <roland at hack dot frob dot com>]

> -	tst-mmap2-eofsync tst-mmap-offend test-fopen-bz12685 bug-fopena+ bug-wfflush \
> +	tst-mmap2-eofsync tst-mmap-offend test-fopen-bz12685 test-fmemopen-bz13152 bug-fopena+ bug-wfflush \

This line is too long (it already was, but fix it now).

> +      if (mode[0] == 'a')
> +	{
> +	  len -= c->maxpos;
> +	  c->buffer += c->maxpos;
> +	  c->maxpos = 0;
> +	}

How can this possibly be right for "a+" mode or ftell?

> +++ b/libio/test-fmemopen-bz12836.c
> @@ -0,0 +1,49 @@
> +/* Test for binary mode see bug 12836.

That's a run-on sentence.  Try:

/* Test for fmemopen binary mode; see BZ#12836.

> +static char buffer[] = "foobar";
> +static char s[]      = "abc";

We don't do ad hoc "alignment" indentation like that.
Put these inside the function.  S can be static const.
BUFFER can just be a stack local.

> +  int i;
> +  FILE *stream;
> +
> +  stream = fmemopen (buffer, strlen (buffer), "w+b");

Use sizeof buffer - 1.

> +	fprintf (stream, s);

Bad indentation here.  Use fputs, not fprintf, for a constant string.

> +  fclose (stream);
> +
> +	for (i = 0 ; i < strlen (s) ; i++)

Use sizeof s - 1 (and no excess space before ; there).

Use C99 style declarations:
  FILE *stream = ...;
  for (int i = 0; ...)


> +  if (buffer[strlen (s)] == '\0')

Use sizeof s - 1.

> +main(int argc, char **argv)

Space before paren, here and throughout.
Use (void) if you don't look at the arguments.

> +  char buf[20] = {"hello, world"};

Drop the braces.

> +  FILE *fp = fmemopen(buf, 20, "a+");

Use sizeof buf.

> +  char c;
> +  int j;

Unused variables.

> +  fseek(fp, 0, SEEK_SET);
> +  fflush(fp);

Maybe check for errors.

> +  fprintf(fp, "X");

Use fputs or fputc.

> +  fclose(fp);
> +  
> +  if (buf[0] != 'h')
> +    return 1;

It's nicer to print something informative in the failure case.


Thanks,
Roland