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

[OndÅej BÃlka <neleai at seznam dot cz>]

On Mon, May 20, 2013 at 10:43:29AM +0200, OndÅej BÃlka wrote:
> On Thu, May 16, 2013 at 05:29:37PM -0700, Roland McGrath wrote:
> > How can this possibly be right for "a+" mode or ftell?
> > 
> ok, need will post new version.
New version. I added append flag and refactored fmemopen a bit.

---
 libio/fmemopen.c              |   74 ++++++++++++++++++----------------------
 libio/test-fmemopen-bz13152.c |   44 ++++++++++++++++++++++++
 2 files changed, 77 insertions(+), 41 deletions(-)
 create mode 100644 libio/test-fmemopen-bz13152.c

diff --git a/libio/fmemopen.c b/libio/fmemopen.c
index 02c764f..d0d2f14 100644
--- a/libio/fmemopen.c
+++ b/libio/fmemopen.c
@@ -82,6 +82,7 @@ struct fmemopen_cookie_struct
   char *buffer;
   int mybuffer;
   int binmode;
+  int append;
   size_t size;
   _IO_off64_t pos;
   size_t maxpos;
@@ -91,9 +92,7 @@ struct fmemopen_cookie_struct
 static ssize_t
 fmemopen_read (void *cookie, char *b, size_t s)
 {
-  fmemopen_cookie_t *c;
-
-  c = (fmemopen_cookie_t *) cookie;
+  fmemopen_cookie_t *c = (fmemopen_cookie_t *) cookie;
 
   if (c->pos + s > c->size)
     {
@@ -115,29 +114,28 @@ fmemopen_read (void *cookie, char *b, size_t s)
 static ssize_t
 fmemopen_write (void *cookie, const char *b, size_t s)
 {
-  fmemopen_cookie_t *c;
   int addnullc;
+  fmemopen_cookie_t *c = (fmemopen_cookie_t *) cookie;
 
-  c = (fmemopen_cookie_t *) cookie;
-
+  _IO_off64_t pos = c->append ? c->maxpos : c->pos;
   addnullc = c->binmode == 0 && (s == 0 || b[s - 1] != '\0');
 
-  if (c->pos + s + addnullc > c->size)
+  if (pos + s + addnullc > c->size)
     {
-      if ((size_t) (c->pos + addnullc) == c->size)
+      if ((size_t) (pos + addnullc) == c->size)
 	{
 	  __set_errno (ENOSPC);
 	  return 0;
 	}
-      s = c->size - c->pos - addnullc;
+      s = c->size - pos - addnullc;
     }
 
-  memcpy (&(c->buffer[c->pos]), b, s);
+  memcpy (&(c->buffer[pos]), b, s);
 
-  c->pos += s;
-  if ((size_t) c->pos > c->maxpos)
+  pos += s;
+  if ((size_t) pos > c->maxpos)
     {
-      c->maxpos = c->pos;
+      c->maxpos = pos;
       if (addnullc)
 	c->buffer[c->maxpos] = '\0';
     }
@@ -150,9 +148,7 @@ static int
 fmemopen_seek (void *cookie, _IO_off64_t *p, int w)
 {
   _IO_off64_t np;
-  fmemopen_cookie_t *c;
-
-  c = (fmemopen_cookie_t *) cookie;
+  fmemopen_cookie_t *c = (fmemopen_cookie_t *) cookie;
 
   switch (w)
     {
@@ -184,9 +180,7 @@ fmemopen_seek (void *cookie, _IO_off64_t *p, int w)
 static int
 fmemopen_close (void *cookie)
 {
-  fmemopen_cookie_t *c;
-
-  c = (fmemopen_cookie_t *) cookie;
+  fmemopen_cookie_t *c = (fmemopen_cookie_t *) cookie;
 
   if (c->mybuffer)
     free (c->buffer);
@@ -204,7 +198,6 @@ fmemopen (void *buf, size_t len, const char *mode)
 
   if (__builtin_expect (len == 0, 0))
     {
-    einval:
       __set_errno (EINVAL);
       return NULL;
     }
@@ -217,39 +210,38 @@ fmemopen (void *buf, size_t len, const char *mode)
 
   if (c->mybuffer)
     {
-      c->buffer = (char *) malloc (len);
-      if (c->buffer == NULL)
+      buf = (char *) malloc (len);
+      if (buf == NULL)
 	{
 	  free (c);
 	  return NULL;
 	}
-      c->buffer[0] = '\0';
-      c->maxpos = 0;
+      buf[0] = '\0';
     }
-  else
+   
+  if (__builtin_expect ((uintptr_t) len > -(uintptr_t) buf, 0))
     {
-      if (__builtin_expect ((uintptr_t) len > -(uintptr_t) buf, 0))
-	{
-	  free (c);
-	  goto einval;
-	}
-
-      c->buffer = buf;
-
-      if (mode[0] == 'w')
-	c->buffer[0] = '\0';
-
-      c->maxpos = strnlen (c->buffer, len);
+      free (c);
+      __set_errno (EINVAL);
+      return NULL;
     }
 
+  if (mode[0] == 'w')
+    buf[0] = '\0';
+
+  c->buffer = buf;
+  c->maxpos = strnlen (c->buffer, len);
   c->size = len;
+  c->pos = 0;
+  c->append = mode[0] == 'a';
+  c->binmode = 0;
 
-  if (mode[0] == 'a')
+  if (mode[0] == 'a' && mode[1] != '+')
     c->pos = c->maxpos;
-  else
-    c->pos = 0;
 
-  c->binmode = mode[0] != '\0' && mode[1] == 'b';
+  for (int i = 0; mode[i]; i++)
+    if (mode[i] == 'b')
+      c->binmode = 1;
 
   iof.read = fmemopen_read;
   iof.write = fmemopen_write;
diff --git a/libio/test-fmemopen-bz13152.c b/libio/test-fmemopen-bz13152.c
new file mode 100644
index 0000000..f2fc5bc
--- /dev/null
+++ b/libio/test-fmemopen-bz13152.c
@@ -0,0 +1,44 @@
+/* Test for binary mode see bug 13152.
+   Copyright (C) 2013 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <string.h>
+
+int
+do_test (int argc, char **argv)
+{
+  char buf[20] = "hello, world";
+  char expected[20] = "hello, worldX";
+  FILE *fp = fmemopen(buf, 20, "a+");
+  fseek(fp, 0, SEEK_SET);
+ 
+  fflush(fp);
+  fprintf(fp, "X");
+  fclose(fp);
+  
+  for (int i = 0; i < strlen (expected); i++)
+    if (buf[i] != expected[i])
+      {
+        printf ("Disagree at position %i\n", i);
+        return 1;
+      }
+
+  return 0;
+}
+
+#include "../test-skeleton.c"
-- 
1.7.4.4