atoll() is a terrible function -- you can't do any error checking with it.

Some of our option processing code uses it.  This means that eg.
'--log-fd=9xxx' logs to fd 9, and '--log-fd=blahblahblah' logs to 0 (because
atoll() returns 0 if the string doesn't contain a number!)

It turns out that most of our option processing uses VG_(strtoll*) instead
of VG_(atoll).  The reason that not all of it does is that the
option-processing macros are underpowered -- they currently work well if you
just want to assign the value to a variable, eg:

        VG_BOOL_CLO(arg, "--heap",   clo_heap)
   else VG_BOOL_CLO(arg, "--stacks", clo_stacks)

   else VG_NUM_CLO(arg, "--heap-admin", clo_heap_admin)
   else VG_NUM_CLO(arg, "--depth",      clo_depth)

(This works because they are actually an if-statement, but it looks odd.)

VG_NUM_CLO uses VG_(stroll10).  But if you want to do any checking or
processing, you can't use those macros, leading to code like this:

      else if (VG_CLO_STREQN(9,  arg, "--log-fd=")) {
         log_to            = VgLogTo_Fd;
         VG_(clo_log_name) = NULL;
         tmp_log_fd        = (Int)VG_(atoll)(&arg[9]);
      }

So this commit:
- Improves the *_CLO_* macros so that they can be used in all circumstances.
  They're now just expressions (albeit ones with side-effects, setting the
  named variable appropriately).  Thus they can be used as if-conditions,
  and any post-checking or processing can occur in the then-statement.  And
  malformed numeric arguments (eg. --log-fd=foo) aren't accepted.  This also
  means you don't have to specify the lengths of any option strings anywhere
  (eg.  the 9 in the --log-fd example above).  The use of a wrong number
  caused at least one bug, in Massif.
- Updates all places where the macros were used.
- Updates Helgrind to use the *_CLO_* macros (it didn't use them).
- Updates Callgrind to use the *_CLO_* macros (it didn't use them), except
  for the more esoteric option names (those with numbers in the option
  name).  This allowed getUInt() and getUWord() to be removed.
- Improves the cache option parsing in Cachegrind and Callgrind -- now uses
  VG_(strtoll10)(), detects overflow, and is shorter.
- Uses INT instead of NUM in the macro names, to distinguish better vs. the
  DBL macro.
- Removes VG_(atoll*) and the few remaining uses -- they're wretched
  functions and VG_(strtoll*) should be used instead.
- Adds the VG_STREQN macro.
- Changes VG_BINT_CLO and VG_BHEX_CLO to abort if the given value is outside
  the range -- the current silent truncation is likely to cause confusion as
  much as anything.


git-svn-id: svn://svn.valgrind.org/valgrind/trunk@9255 a5019735-40e9-0310-863c-91ae7b9d1cf9

1 files changed, 20 insertions, 37 deletions

diff --git a/massif/ms_main.c b/massif/ms_main.c
index 32b6eeca7..2d8c26a05 100644
--- a/massif/ms_main.c
+++ b/massif/ms_main.c
@@ -358,46 +358,45 @@ static Bool clo_heap            = True;
    // word-sized type -- it ended up with a value of 4.2 billion.  Sigh.
 static SSizeT clo_heap_admin      = 8;
 static Bool   clo_stacks          = False;
-static UInt   clo_depth           = 30;
+static Int    clo_depth           = 30;
 static double clo_threshold       = 1.0;  // percentage
 static double clo_peak_inaccuracy = 1.0;  // percentage
-static UInt   clo_time_unit       = TimeI;
-static UInt   clo_detailed_freq   = 10;
-static UInt   clo_max_snapshots   = 100;
+static Int    clo_time_unit       = TimeI;
+static Int    clo_detailed_freq   = 10;
+static Int    clo_max_snapshots   = 100;
 static Char*  clo_massif_out_file = "massif.out.%p";
 
 static XArray* args_for_massif;
 
 static Bool ms_process_cmd_line_option(Char* arg)
 {
+   Char* tmp_str;
+
    // Remember the arg for later use.
    VG_(addToXA)(args_for_massif, &arg);
 
-        VG_BOOL_CLO(arg, "--heap",   clo_heap)
-   else VG_BOOL_CLO(arg, "--stacks", clo_stacks)
+        if VG_BOOL_CLO(arg, "--heap",   clo_heap)   {}
+   else if VG_BOOL_CLO(arg, "--stacks", clo_stacks) {}
 
-   else VG_NUM_CLO(arg, "--heap-admin", clo_heap_admin)
-   else VG_NUM_CLO(arg, "--depth",      clo_depth)
+   else if VG_BINT_CLO(arg, "--heap-admin", clo_heap_admin, 0, 1024) {}
+   else if VG_BINT_CLO(arg, "--depth",      clo_depth, 1, MAX_DEPTH) {}
 
-   else VG_DBL_CLO(arg, "--threshold",  clo_threshold)
+   else if VG_DBL_CLO(arg, "--threshold",  clo_threshold) {}
 
-   else VG_DBL_CLO(arg, "--peak-inaccuracy", clo_peak_inaccuracy)
+   else if VG_DBL_CLO(arg, "--peak-inaccuracy", clo_peak_inaccuracy) {}
 
-   else VG_NUM_CLO(arg, "--detailed-freq", clo_detailed_freq)
-   else VG_NUM_CLO(arg, "--max-snapshots", clo_max_snapshots)
+   else if VG_BINT_CLO(arg, "--detailed-freq", clo_detailed_freq, 1, 10000) {}
+   else if VG_BINT_CLO(arg, "--max-snapshots", clo_max_snapshots, 10, 1000) {}
 
-   else if (VG_CLO_STREQ(arg, "--time-unit=i"))  clo_time_unit = TimeI;
-   else if (VG_CLO_STREQ(arg, "--time-unit=ms")) clo_time_unit = TimeMS;
-   else if (VG_CLO_STREQ(arg, "--time-unit=B"))  clo_time_unit = TimeB;
+   else if VG_XACT_CLO(arg, "--time-unit=i",  clo_time_unit, TimeI)  {}
+   else if VG_XACT_CLO(arg, "--time-unit=ms", clo_time_unit, TimeMS) {}
+   else if VG_XACT_CLO(arg, "--time-unit=B",  clo_time_unit, TimeB)  {}
 
-   else if (VG_CLO_STREQN(11, arg, "--alloc-fn=")) {
-      Char* alloc_fn = &arg[11];
-      VG_(addToXA)(alloc_fns, &alloc_fn);
+   else if VG_STR_CLO(arg, "--alloc-fn", tmp_str) {
+      VG_(addToXA)(alloc_fns, &tmp_str);
    }
 
-   else if (VG_CLO_STREQN(18, arg, "--massif-out-file=")) {
-      clo_massif_out_file = &arg[18];
-   }
+   else if VG_STR_CLO(arg, "--massif-out-file", clo_massif_out_file) {}
 
    else
       return VG_(replacement_malloc_process_cmd_line_option)(arg);
@@ -2164,26 +2163,10 @@ static void ms_post_clo_init(void)
    Int i;
 
    // Check options.
-   if (clo_heap_admin < 0 || clo_heap_admin > 1024) {
-      VG_(message)(Vg_UserMsg, "--heap-admin must be between 0 and 1024");
-      VG_(err_bad_option)("--heap-admin");
-   }
-   if (clo_depth < 1 || clo_depth > MAX_DEPTH) {
-      VG_(message)(Vg_UserMsg, "--depth must be between 1 and %d", MAX_DEPTH);
-      VG_(err_bad_option)("--depth");
-   }
    if (clo_threshold < 0 || clo_threshold > 100) {
       VG_(message)(Vg_UserMsg, "--threshold must be between 0.0 and 100.0");
       VG_(err_bad_option)("--threshold");
    }
-   if (clo_detailed_freq < 1 || clo_detailed_freq > 10000) {
-      VG_(message)(Vg_UserMsg, "--detailed-freq must be between 1 and 10000");
-      VG_(err_bad_option)("--detailed-freq");
-   }
-   if (clo_max_snapshots < 10 || clo_max_snapshots > 1000) {
-      VG_(message)(Vg_UserMsg, "--max-snapshots must be between 10 and 1000");
-      VG_(err_bad_option)("--max-snapshots");
-   }
 
    // If we have --heap=no, set --heap-admin to zero, just to make sure we
    // don't accidentally use a non-zero heap-admin size somewhere.