Subject: Re: "du -b --files0-from=-" running out of memory
From: Jim Meyering <[email-masked]>
To: Pádraig Brady <[email-masked]>
Cc: Eric Blake <[email-masked]>, [email-masked], bug-coreutils <[email-masked]>
Date: Tue, 25 Nov 2008 18:41:18 +0100
List-Id: <cygwin.cygwin.com>

Pádraig Brady <[email-masked]> wrote:
>>> I notice that argv_iter does a malloc() + memcpy() per entry.
>>> Since the sources are already NUL terminated strings
>>> perhaps it could just return a pointer to a getdelim
>>> realloc'd buffer which was referenced in the argv_iterator struct.
>>
>> The only per-entry allocation I see is:
>>   - in argv-mode: strdup
>>   - in stream-reading mode: getdelim
>>
>> Did I miss something?
>
> Sorry I was unclear.
> strdup() and getdelim(NULL,...) do a malloc() + memcpy()
> if you maintain the char* name and size_t buf_len in
> the argv_iterator struct, then you can return pointers
> to the orig data, and remove the need to free() from the
> users of argv_iter().

Good suggestion!  Thanks.

diff --git a/gl/lib/argv-iter.c b/gl/lib/argv-iter.c
index 204769a..adaff6f 100644
--- a/gl/lib/argv-iter.c
+++ b/gl/lib/argv-iter.c
@@ -28,6 +28,8 @@ struct argv_iterator
   /* file-mode: fp records position */
   FILE *fp;
   size_t item_idx;
+  char *tok;
+  size_t buf_len;

   /* argv-mode: record just argv and current pointer */
   char **arg_list;
@@ -55,6 +57,9 @@ argv_iter_init_stream (FILE *fp)
   if (!ai)
     return NULL;
   ai->fp = fp;
+  ai->tok = NULL;
+  ai->buf_len = 0;
+
   ai->item_idx = 0;
   ai->arg_list = NULL;
   return ai;
@@ -65,19 +70,16 @@ argv_iter (struct argv_iterator *ai, enum argv_iter_err *err)
 {
   if (ai->fp)
     {
-      char *name = NULL;
-      size_t buf_len = 0;
-      ssize_t len = getdelim (&name, &buf_len, '\0', ai->fp);
+      ssize_t len = getdelim (&ai->tok, &ai->buf_len, '\0', ai->fp);
       if (len < 0)
         {
-          free (name);
           *err = feof (ai->fp) ? AI_ERR_EOF : AI_ERR_READ;
           return NULL;
         }

       *err = AI_ERR_OK;
       ai->item_idx++;
-      return name;
+      return ai->tok;
     }
   else
     {
@@ -89,7 +91,7 @@ argv_iter (struct argv_iterator *ai, enum argv_iter_err *err)
       else
         {
           *err = AI_ERR_OK;
-          return strdup (*(ai->p++));
+          return *(ai->p++);
         }
     }
 }
@@ -103,6 +105,8 @@ argv_iter_n_args (struct argv_iterator const *ai)
 void
 argv_iter_free (struct argv_iterator *ai)
 {
+  if (ai->fp)
+    free (ai->tok);
   free (ai);
 }

diff --git a/src/du.c b/src/du.c
index 5ed2b12..a1345a7 100644
--- a/src/du.c
+++ b/src/du.c
@@ -968,7 +968,7 @@ main (int argc, char **argv)
 	    case AI_ERR_READ:
 	      error (0, errno, _("%s: read error"), quote (files_from));
 	      skip_file = true;
-	      goto next_file_name;
+	      continue;

 	    case AI_ERR_MEM:
 	      xalloc_die ();
@@ -1016,9 +1016,6 @@ main (int argc, char **argv)
 	  temp_argv[0] = file_name;
 	  ok &= du_files (temp_argv, bit_flags);
 	}
-
-    next_file_name:;
-      free (file_name);
     }

   argv_iter_free (ai);

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Problem reports:       http://cygwin.com/problems.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/