From: Florian Krohm <florian@eich-krohm.de>
Date: Sat, 29 Nov 2014 13:31:18 +0000 (+0000)
Subject: Fix up the error processing in VG_(expand_file_name). E.g. giving
X-Git-Tag: svn/VALGRIND_3_11_0~792
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2ad0d731f9b1ed33afd0fe5ad8fc2380262be236;p=thirdparty%2Fvalgrind.git

Fix up the error processing in VG_(expand_file_name). E.g. giving
--log-file=  on the command line results in the following error:

valgrind: --log-file: filename is emptyBad option: --log-file=
...

Relatedly, fix the 1st argument to VG_(expand_file_name) in coredump-elf.c.
This should not contain additional verbiage as it is assumed to be an option
name which us used to construct an error message containing
option_name=file_name

As an aside, this logic in coredump-elf.c seems odd:
If VG_(clo_log_fname_expanded) is not NULL, then it has already been
expanded in main_process_cmd_line_options. Expanding it again would only
make a difference, if the original logfile name contained an environment
variable whose value contained %q{whatever} thereby referring to a yet
another environment variable. That seems strange.
But I'm not touching it.


git-svn-id: svn://svn.valgrind.org/valgrind/trunk@14794
---

diff --git a/coregrind/m_coredump/coredump-elf.c b/coregrind/m_coredump/coredump-elf.c
index 4990141b86..f0034ccacd 100644
--- a/coregrind/m_coredump/coredump-elf.c
+++ b/coregrind/m_coredump/coredump-elf.c
@@ -599,9 +599,8 @@ void make_elf_coredump(ThreadId tid, const vki_siginfo_t *si, ULong max_size)
 
    if (VG_(clo_log_fname_expanded) != NULL) {
       coreext = ".core";
-      basename = VG_(expand_file_name)(
-                    "--log-file (while creating core filename)",
-                    VG_(clo_log_fname_expanded));
+      basename = VG_(expand_file_name)("--log-file",
+                                       VG_(clo_log_fname_expanded));
    }
 
    vg_assert(coreext);
diff --git a/coregrind/m_options.c b/coregrind/m_options.c
index 5284f2552f..1238ca1cad 100644
--- a/coregrind/m_options.c
+++ b/coregrind/m_options.c
@@ -150,12 +150,13 @@ HChar* VG_(expand_file_name)(const HChar* option_name, const HChar* format)
    const HChar *base_dir;
    Int len, i = 0, j = 0;
    HChar* out;
+   const HChar *message = NULL;
 
    base_dir = VG_(get_startup_wd)();
 
    if (VG_STREQ(format, "")) {
       // Empty name, bad.
-      VG_(fmsg)("%s: filename is empty", option_name);
+      message = "No filename given\n";
       goto bad;
    }
    
@@ -164,13 +165,11 @@ HChar* VG_(expand_file_name)(const HChar* option_name, const HChar* format)
    // that we don't allow a legitimate filename beginning with '~' but that
    // seems very unlikely.
    if (format[0] == '~') {
-      VG_(fmsg)(
-         "%s: filename begins with '~'\n"
+      message = 
+         "Filename begins with '~'\n"
          "You probably expected the shell to expand the '~', but it\n"
          "didn't.  The rules for '~'-expansion vary from shell to shell.\n"
-         "You might have more luck using $HOME instead.\n",
-         option_name
-      );
+         "You might have more luck using $HOME instead.\n";
       goto bad;
    }
 
@@ -212,7 +211,7 @@ HChar* VG_(expand_file_name)(const HChar* option_name, const HChar* format)
                Int begin_qualname = ++i;
                while (True) {
                   if (0 == format[i]) {
-                     VG_(fmsg)("%s: malformed %%q specifier\n", option_name);
+                     message = "Missing '}' in %q specifier\n";
                      goto bad;
                   } else if ('}' == format[i]) {
                      Int qualname_len = i - begin_qualname;
@@ -222,8 +221,14 @@ HChar* VG_(expand_file_name)(const HChar* option_name, const HChar* format)
                      qualname[qualname_len] = '\0';
                      qual = VG_(getenv)(qualname);
                      if (NULL == qual) {
-                        VG_(fmsg)("%s: environment variable %s is not set\n",
-                                  option_name, qualname);
+                        // This memory will leak, But we don't care because
+                        // VG_(fmsg_bad_option) will terminate the process.
+                        HChar *str = VG_(malloc)("options.efn.3",
+                                                 100 + qualname_len);
+                        VG_(sprintf)(str,
+                                     "Environment variable '%s' is not set\n",
+                                     qualname);
+                        message = str;
                         goto bad;
                      }
                      i++;
@@ -234,14 +239,13 @@ HChar* VG_(expand_file_name)(const HChar* option_name, const HChar* format)
                ENSURE_THIS_MUCH_SPACE(VG_(strlen)(qual));
                j += VG_(sprintf)(&out[j], "%s", qual);
             } else {
-               VG_(fmsg)("%s: expected '{' after '%%q'\n", option_name);
+               message = "Expected '{' after '%q'\n";
                goto bad;
             }
          } 
          else {
             // Something else, abort.
-            VG_(fmsg)("%s: expected 'p' or 'q' or '%%' after '%%'\n",
-                      option_name);
+            message = "Expected 'p' or 'q' or '%' after '%'\n";
             goto bad;
          }
       }
@@ -264,13 +268,11 @@ HChar* VG_(expand_file_name)(const HChar* option_name, const HChar* format)
    return out;
 
   bad: {
-   HChar* opt =    // 2:  1 for the '=', 1 for the NUL.
-      VG_(malloc)( "options.efn.3",
-                   VG_(strlen)(option_name) + VG_(strlen)(format) + 2 );
-   VG_(strcpy)(opt, option_name);
-   VG_(strcat)(opt, "=");
-   VG_(strcat)(opt, format);
-   VG_(fmsg_bad_option)(opt, "");
+   vg_assert(message != NULL);
+   // 2:  1 for the '=', 1 for the NUL.
+   HChar opt[VG_(strlen)(option_name) + VG_(strlen)(format) + 2];
+   VG_(sprintf)(opt, "%s=%s", option_name, format);
+   VG_(fmsg_bad_option)(opt, "%s", message);
   }
 }