On 08/18/2010 05:43 AM, Giuseppe Scrivano wrote:
PING^2
are there more problems with this patch?
Subject: [PATCH] read-file: Avoid memory reallocations with regular files.
* modules/read-file (Depends-on): Add ftello and malloc-posix.
* lib/read-file.c: Include<sys/types.h>,<sys/stat.h>,
<unistd.h>,<stdio.h>,<stdint.h>.
(fread_file): With regular files, use the remaining length as the
initial buffer size. Check against overflow.
I see a few changes to squash in:
+/* Get fstat. */
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
POSIX (and therefore gnulib) guarantees that <sys/stat.h> is sufficient
for fstat without explicitly listing <sys/types.h>. And I didn't see
any use of <unistd.h> in your additions.
+
+/* Get ftello. */
+#include <stdio.h>
+
+/* Get SIZE_MAX. */
+#include <stdint.h>
+
/* Get realloc, free. */
#include <stdlib.h>
@@ -38,6 +49,33 @@ fread_file (FILE * stream, size_t * length)
size_t alloc = 0;
size_t size = 0;
int save_errno;
+ struct stat st;
+
+ do
+ {
+ off_t alloc_off, pos;
GNU style prefers listing each declaration on a separate line.
Also, tests/test-read-file should probably add a test for reading a
FILE* that wraps a pipe in addition to its current tests of seekable
files, to make sure we don't regress on non-seekable files (perhaps by
adding test-read-file.sh, and making test-read-file.c consume stdin for
several file types tied to stdin).
More problematic is this, which prevented me from pushing your patch:
> gnulib-tool: warning: module read-file depends on a module with an
incompatible license: ftello
Before your change can be applied, we'd need consensus that ftello
should be LGPLv2+ instead of its current LGPLv3+. I think that boils
down to just Bruno and myself, and I'm okay with the idea.
diff --git i/lib/read-file.c w/lib/read-file.c
index 628def1..34156bf 100644
--- i/lib/read-file.c
+++ w/lib/read-file.c
@@ -21,9 +21,7 @@
#include "read-file.h"
/* Get fstat. */
-#include <sys/types.h>
#include <sys/stat.h>
-#include <unistd.h>
/* Get ftello. */
#include <stdio.h>
@@ -53,7 +51,8 @@ fread_file (FILE * stream, size_t * length)
do
{
- off_t alloc_off, pos;
+ off_t alloc_off;
+ off_t pos;
if (fstat (fileno (stream), &st) < 0 || !S_ISREG (st.st_mode))
break;
--
Eric Blake ebl...@redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
