Re: [PATCH] Use strdup in dd to avoid changing argv elements

[Jim Meyering]

Eric Blake <address@hidden> wrote:

> According to Jim Meyering on 1/28/2008 6:28 AM:
> |
> | Thanks for the suggestion, but that introduces a new way for
> | dd to fail: strdup returning NULL would often lead to a segfault.
> | Even if it were to use xstrdup, to avoid that, I don't think it's
> | justifiable solely in order to preserve ps' view of the dd command line.
>
> However, it IS justifiable by the fact that POSIX requires applications to
> treat argv[] as constant (ie. modifying argv in-place is not
> standards-compliant).  True, most systems let you get away with
> modifications (in particular, think about getopt_long's behavior), but it
> would matter if we ever port coreutils to a system that follows the POSIX
> permission to insist on no modifications to the original argv and contents.
>
> http://www.opengroup.org/onlinepubs/009695399/functions/execl.html
> "The statement about argv[] and envp[] being constants is included to make
> explicit to future writers of language bindings that these objects are
> completely constant. Due to a limitation of the ISO C standard, it is not
> possible to state that idea in standard C."

Good point.  Thanks.
Here's the snap patch I'm now considering.
It has nominal leaks only for certain invalid arguments.

diff --git a/src/dd.c b/src/dd.c
index cc1ba0c..98b0ea5 100644
--- a/src/dd.c
+++ b/src/dd.c
@@ -1,5 +1,5 @@
 /* dd -- convert a file while copying it.
-   Copyright (C) 85, 90, 91, 1995-2007 Free Software Foundation, Inc.
+   Copyright (C) 85, 90, 91, 1995-2008 Free Software Foundation, Inc.

    This program is free software: you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -31,6 +31,7 @@
 #include "human.h"
 #include "long-options.h"
 #include "quote.h"
+#include "xstrndup.h"
 #include "xstrtol.h"
 #include "xtime.h"

@@ -877,14 +878,14 @@ scanargs (int argc, char **argv)
     {
       char *name, *val;

-      name = argv[i];
-      val = strchr (name, '=');
+      val = strchr (argv[i], '=');
       if (val == NULL)
        {
-         error (0, 0, _("unrecognized operand %s"), quote (name));
+         error (0, 0, _("unrecognized operand %s"), quote (argv[i]));
          usage (EXIT_FAILURE);
        }
-      *val++ = '\0';
+      name = xstrndup (argv[i], val - argv[i]);
+      val++;

       if (STREQ (name, "if"))
        input_file = val;
@@ -945,6 +946,7 @@ scanargs (int argc, char **argv)
          if (invalid)
            error (EXIT_FAILURE, 0, _("invalid number %s"), quote (val));
        }
+      free (name);
     }

   if (blocksize)