This is the mail archive of the libc-hacker@sources.redhat.com mailing list for the glibc project.

Note that libc-hacker is a closed list. You may look at the archives of this list, but subscription and posting are not open.


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

[PATCH] Fix mmap stdio vs. set*buf* interaction


Hi!

See http://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=65864
Basically, f = fopen (somefile, "r"); setvbuf (f, buf, _IOFBF, sizeof buf);
will result in broken stream.
I see 3 possible solutions:
1) (attached below): Make _IO_SETBUF and _IO_WSETBUF on mmap stdio
   pretend it did something, yet don't do anything.
   This will make apps happy if they e.g. are checking setvbuf return value,
   but on the other side one cannot rely on __fbufsize(f) returning the size
   last setvbuf gave it.
2) Make _IO_*SETBUF on mmap stdio return NULL. This would mean setvbuf etc.
   would return error code and if apps check whether it succeeded and bail
   if not, this would kill them. But __fbusize(f) would reflect the last
   successful setvbuf.
3) When _IO_*SETBUF is called on mmap stdio stream, first change the stream
   back to normal non-mmap one and then actually set the buffer.
   IMHO this would be fairly complicated to do and I don't know if there
   would be enough applications which would benefit from this.
What do you think?

2002-06-05  Jakub Jelinek  <jakub@redhat.com>

	* libio/fileops.c (_IO_file_setbuf_mmap): New.
	(_IO_file_jumps_mmap): Use it.
	* libio/wfileops.c (_IO_wfile_setbuf_mmap): New.
	(_IO_wfile_jumps_mmap): Use it.
	* libio/Makefile (tests): Add tst-mmap-setvbuf.
	* libio/tst-mmap-setvbuf.c: New test.

--- libc/libio/fileops.c.jj	Sun Apr  7 17:46:18 2002
+++ libc/libio/fileops.c	Wed Jun  5 12:07:43 2002
@@ -440,6 +440,19 @@ _IO_new_file_setbuf (fp, p, len)
 }
 INTDEF2(_IO_new_file_setbuf, _IO_file_setbuf)
 
+static _IO_FILE * _IO_file_setbuf_mmap __P ((_IO_FILE *, char *, _IO_ssize_t));
+
+static _IO_FILE *
+_IO_file_setbuf_mmap (fp, p, len)
+     _IO_FILE *fp;
+     char *p;
+     _IO_ssize_t len;
+{
+  /* Changing the buffer for mmap stdio is not possible
+     (unless the stream is reverted to non-mmap stdio).  */
+  return fp;
+}
+
 static int new_do_write __P ((_IO_FILE *, const char *, _IO_size_t));
 
 /* Write TO_DO bytes from DATA to FP.
@@ -1293,7 +1306,7 @@ struct _IO_jump_t _IO_file_jumps_mmap =
   JUMP_INIT(xsgetn, _IO_file_xsgetn_mmap),
   JUMP_INIT(seekoff, _IO_file_seekoff_mmap),
   JUMP_INIT(seekpos, _IO_default_seekpos),
-  JUMP_INIT(setbuf, _IO_new_file_setbuf),
+  JUMP_INIT(setbuf, _IO_file_setbuf_mmap),
   JUMP_INIT(sync, _IO_new_file_sync),
   JUMP_INIT(doallocate, INTUSE(_IO_file_doallocate)),
   JUMP_INIT(read, INTUSE(_IO_file_read)),
--- libc/libio/Makefile.jj	Wed Jun  5 10:27:28 2002
+++ libc/libio/Makefile	Wed Jun  5 12:10:51 2002
@@ -49,7 +49,7 @@ routines	:=							      \
 tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc   \
 	tst_wprintf2 tst-widetext test-fmemopen tst-ext tst-fopenloc	      \
 	tst-fgetws tst-ungetwc1 tst-ungetwc2 tst-swscanf tst-sscanf	      \
-	bug-ungetwc1 bug-ungetwc2
+	tst-mmap-setvbuf bug-ungetwc1 bug-ungetwc2
 test-srcs = test-freopen
 
 all: # Make this the default target; it will be defined in Rules.
--- libc/libio/wfileops.c.jj	Sat Mar 23 11:50:27 2002
+++ libc/libio/wfileops.c	Wed Jun  5 12:07:43 2002
@@ -70,6 +70,21 @@ _IO_wfile_setbuf (fp, p, len)
 }
 
 
+static _IO_FILE * _IO_wfile_setbuf_mmap __P ((_IO_FILE *, wchar_t *,
+					      _IO_ssize_t));
+
+static _IO_FILE *
+_IO_wfile_setbuf_mmap (fp, p, len)
+     _IO_FILE *fp;
+     wchar_t *p;
+     _IO_ssize_t len;
+{
+  /* Changing the buffer for mmap stdio is not possible
+     (unless the stream is reverted to non-mmap stdio).  */
+  return fp;
+}
+
+
 /* Convert TO_DO wide character from DATA to FP.
    Then mark FP as having empty buffers. */
 int
@@ -877,7 +892,7 @@ struct _IO_jump_t _IO_wfile_jumps_mmap =
   JUMP_INIT(xsgetn, INTUSE(_IO_file_xsgetn)),
   JUMP_INIT(seekoff, INTUSE(_IO_wfile_seekoff)),
   JUMP_INIT(seekpos, _IO_default_seekpos),
-  JUMP_INIT(setbuf, _IO_new_file_setbuf),
+  JUMP_INIT(setbuf, _IO_wfile_setbuf_mmap),
   JUMP_INIT(sync, (_IO_sync_t) INTUSE(_IO_wfile_sync)),
   JUMP_INIT(doallocate, _IO_wfile_doallocate),
   JUMP_INIT(read, INTUSE(_IO_file_read)),
--- libc/libio/tst-mmap-setvbuf.c.jj	Wed Jun  5 12:07:51 2002
+++ libc/libio/tst-mmap-setvbuf.c	Wed Jun  5 12:10:00 2002
@@ -0,0 +1,80 @@
+/* Test setvbuf on readonly fopen (using mmap stdio).
+   Copyright (C) 2002 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+   Contributed by Jakub Jelinek <jakub@redhat.com>, 2002.
+
+   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, write to the Free
+   Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+   02111-1307 USA.  */
+
+#include <stdio.h>
+#include <unistd.h>
+
+int main (void)
+{
+  char name[] = "/tmp/tst-mmap-setvbuf.XXXXXX";
+  char buf[4096];
+  const char * const test = "Let's see if mmap stdio works with setvbuf.\n";
+  char temp[strlen (test) + 1];
+  int fd = mkstemp (name);
+  FILE *f;
+
+  if (fd == -1)
+    {
+      printf ("%Zd: cannot open temporary file: %m\n", __LINE__);
+      exit (1);
+    }
+
+  f = fdopen (fd, "w");
+  if (f == NULL)
+    {
+      printf ("%Zd: cannot fdopen temporary file: %m\n", __LINE__);
+      exit (1);
+    }
+
+  fputs (test, f);
+  fclose (f);
+
+  f = fopen (name, "r");
+  if (f == NULL)
+    {
+      printf ("%Zd: cannot fopen temporary file: %m\n", __LINE__);
+      exit (1);
+    }
+
+  if (setvbuf (f, buf, _IOFBF, sizeof buf))
+    {
+      printf ("%Zd: setvbuf failed: %m\n", __LINE__);
+      exit (1);
+    }
+
+  if (fread (temp, 1, strlen (test), f) != strlen (test))
+    {
+      printf ("%Zd: couldn't read the file back: %m\n", __LINE__);
+      exit (1);
+    }
+  temp [strlen (test)] = '\0';
+
+  if (strcmp (test, temp))
+    {
+      printf ("%Zd: read different string than was written:\n%s%s",
+	      __LINE__, test, temp);
+      exit (1);
+    }
+
+  fclose (f);
+
+  unlink (name);
+  exit (0);
+}

	Jakub


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