This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v2][BZ #13152] fmemopen does not honor append mode.
- 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: Thu, 16 May 2013 17:29:37 -0700 (PDT)
- Subject: Re: [PATCH v2][BZ #13152] fmemopen does not honor append mode.
- References: <20130509164726 dot GA32347 at domone dot kolej dot mff dot cuni dot cz> <Pine dot LNX dot 4 dot 64 dot 1305091653510 dot 25142 at digraph dot polyomino dot org dot uk> <20130514131101 dot GA3860 at domone dot kolej dot mff dot cuni dot cz>
> - 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