Menu
▾
▴
[Valgrind-developers] valgrind: r11209 - in trunk: cachegrind callgrind coregrind coregrind/m_replacemalloc coregrind/m_ume include massif memcheck none/tests
|
From: <sv...@va...> - 2010-07-06 04:05:38
|
Author: njn
Date: 2010-07-06 05:05:23 +0100 (Tue, 06 Jul 2010)
New Revision: 11209
Log:
Make error messages at start-up more consistent. Every line of such
messages now begin with "valgrind: ", and they're more often printed before
the preamble. This required introducing a new message kind, Vg_FailMsg, and
functions VG_(fmsg) and VG_(fmsg_bad_option), and removing
VG_(err_bad_option).
Where we used to have horrible output like this:
[ocean:~/grind/ws2] vg5 --tool=massif --threshold=101 date
==31877== Massif, a heap profiler
==31877== Copyright (C) 2003-2010, and GNU GPL'd, by Nicholas Nethercote
==31877== Using Valgrind-3.6.0.SVN and LibVEX; rerun with -h for copyright info
==31877== Command: date
==31877==
==31877== --threshold must be between 0.0 and 100.0
valgrind: Bad option '--threshold'; aborting.
valgrind: Use --help for more information.
We now have nice output like this:
[ocean:~/grind/ws2] vg2 --tool=massif --threshold=101 date
valgrind: Bad option: --threshold=101
valgrind: --threshold must be between 0.0 and 100.0
valgrind: Use --help for more information or consult the user manual.
Modified:
trunk/cachegrind/cg_main.c
trunk/callgrind/sim.c
trunk/coregrind/m_libcprint.c
trunk/coregrind/m_main.c
trunk/coregrind/m_options.c
trunk/coregrind/m_replacemalloc/replacemalloc_core.c
trunk/coregrind/m_ume/main.c
trunk/coregrind/pub_core_libcprint.h
trunk/coregrind/pub_core_options.h
trunk/include/pub_tool_libcprint.h
trunk/include/pub_tool_options.h
trunk/include/pub_tool_tooliface.h
trunk/massif/ms_main.c
trunk/memcheck/mc_main.c
trunk/none/tests/cmdline2.stdout.exp
trunk/none/tests/cmdline4.stderr.exp
Modified: trunk/cachegrind/cg_main.c
===================================================================
--- trunk/cachegrind/cg_main.c 2010-07-05 07:21:22 UTC (rev 11208)
+++ trunk/cachegrind/cg_main.c 2010-07-06 04:05:23 UTC (rev 11209)
@@ -1191,45 +1191,40 @@
static cache_t clo_D1_cache = UNDEFINED_CACHE;
static cache_t clo_L2_cache = UNDEFINED_CACHE;
-/* Checks cache config is ok; makes it so if not. */
-static
-void check_cache(cache_t* cache, Char *name)
+// Checks cache config is ok. Returns NULL if ok, or a pointer to an error
+// string otherwise.
+static Char* check_cache(cache_t* cache)
{
- /* Simulator requires line size and set count to be powers of two */
- if (( cache->size % (cache->line_size * cache->assoc) != 0) ||
- (-1 == VG_(log2)(cache->size/cache->line_size/cache->assoc))) {
- VG_(umsg)("error: %s set count not a power of two; aborting.\n", name);
- VG_(exit)(1);
+ // Simulator requires set count to be a power of two.
+ if ((cache->size % (cache->line_size * cache->assoc) != 0) ||
+ (-1 == VG_(log2)(cache->size/cache->line_size/cache->assoc)))
+ {
+ return "Cache set count is not a power of two.\n";
}
+ // Simulator requires line size to be a power of two.
if (-1 == VG_(log2)(cache->line_size)) {
- VG_(umsg)("error: %s line size of %dB not a power of two; aborting.\n",
- name, cache->line_size);
- VG_(exit)(1);
+ return "Cache line size is not a power of two.\n";
}
// Then check line size >= 16 -- any smaller and a single instruction could
// straddle three cache lines, which breaks a simulation assertion and is
// stupid anyway.
if (cache->line_size < MIN_LINE_SIZE) {
- VG_(umsg)("error: %s line size of %dB too small; aborting.\n",
- name, cache->line_size);
- VG_(exit)(1);
+ return "Cache line size is too small.\n";
}
/* Then check cache size > line size (causes seg faults if not). */
if (cache->size <= cache->line_size) {
- VG_(umsg)("error: %s cache size of %dB <= line size of %dB; aborting.\n",
- name, cache->size, cache->line_size);
- VG_(exit)(1);
+ return "Cache size <= line size.\n";
}
/* Then check assoc <= (size / line size) (seg faults otherwise). */
if (cache->assoc > (cache->size / cache->line_size)) {
- VG_(umsg)("warning: %s associativity > (size / line size); aborting.\n",
- name);
- VG_(exit)(1);
+ return "Cache associativity > (size / line size).\n";
}
+
+ return NULL;
}
static
@@ -1237,27 +1232,29 @@
{
#define DEFINED(L) (-1 != L.size || -1 != L.assoc || -1 != L.line_size)
- Int n_clos = 0;
+ Char* checkRes;
// Count how many were defined on the command line.
- if (DEFINED(clo_I1_cache)) { n_clos++; }
- if (DEFINED(clo_D1_cache)) { n_clos++; }
- if (DEFINED(clo_L2_cache)) { n_clos++; }
+ Bool all_caches_clo_defined =
+ (DEFINED(clo_I1_cache) &&
+ DEFINED(clo_D1_cache) &&
+ DEFINED(clo_L2_cache));
// Set the cache config (using auto-detection, if supported by the
- // architecture)
- VG_(configure_caches)( I1c, D1c, L2c, (3 == n_clos) );
+ // architecture).
+ VG_(configure_caches)( I1c, D1c, L2c, all_caches_clo_defined );
- // Then replace with any defined on the command line.
+ // Check the default/auto-detected values.
+ checkRes = check_cache(I1c); tl_assert(!checkRes);
+ checkRes = check_cache(D1c); tl_assert(!checkRes);
+ checkRes = check_cache(L2c); tl_assert(!checkRes);
+
+ // Then replace with any defined on the command line. (Already checked in
+ // parse_cache_opt().)
if (DEFINED(clo_I1_cache)) { *I1c = clo_I1_cache; }
if (DEFINED(clo_D1_cache)) { *D1c = clo_D1_cache; }
if (DEFINED(clo_L2_cache)) { *L2c = clo_L2_cache; }
- // Then check values and fix if not acceptable.
- check_cache(I1c, "I1");
- check_cache(D1c, "D1");
- check_cache(L2c, "L2");
-
if (VG_(clo_verbosity) >= 2) {
VG_(umsg)("Cache configuration used:\n");
VG_(umsg)(" I1: %dB, %d-way, %dB lines\n",
@@ -1671,13 +1668,14 @@
/*--- Command line processing ---*/
/*--------------------------------------------------------------------*/
-static void parse_cache_opt ( cache_t* cache, Char* opt )
+static void parse_cache_opt ( cache_t* cache, Char* opt, Char* optval )
{
Long i1, i2, i3;
Char* endptr;
+ Char* checkRes;
// Option argument looks like "65536,2,64". Extract them.
- i1 = VG_(strtoll10)(opt, &endptr); if (*endptr != ',') goto bad;
+ i1 = VG_(strtoll10)(optval, &endptr); if (*endptr != ',') goto bad;
i2 = VG_(strtoll10)(endptr+1, &endptr); if (*endptr != ',') goto bad;
i3 = VG_(strtoll10)(endptr+1, &endptr); if (*endptr != '\0') goto bad;
@@ -1689,14 +1687,20 @@
if (cache->assoc != i2) goto overflow;
if (cache->line_size != i3) goto overflow;
+ checkRes = check_cache(cache);
+ if (checkRes) {
+ VG_(fmsg)("%s", checkRes);
+ goto bad;
+ }
+
return;
+ bad:
+ VG_(fmsg_bad_option)(opt, "");
+
overflow:
- VG_(umsg)("one of the cache parameters was too large and overflowed\n");
- bad:
- // XXX: this omits the "--I1/D1/L2=" part from the message, but that's
- // not a big deal.
- VG_(err_bad_option)(opt);
+ VG_(fmsg_bad_option)(opt,
+ "One of the cache parameters was too large and overflowed.\n");
}
static Bool cg_process_cmd_line_option(Char* arg)
@@ -1705,11 +1709,11 @@
// 5 is length of "--I1="
if VG_STR_CLO(arg, "--I1", tmp_str)
- parse_cache_opt(&clo_I1_cache, tmp_str);
+ parse_cache_opt(&clo_I1_cache, arg, tmp_str);
else if VG_STR_CLO(arg, "--D1", tmp_str)
- parse_cache_opt(&clo_D1_cache, tmp_str);
+ parse_cache_opt(&clo_D1_cache, arg, tmp_str);
else if VG_STR_CLO(arg, "--L2", tmp_str)
- parse_cache_opt(&clo_L2_cache, tmp_str);
+ parse_cache_opt(&clo_L2_cache, arg, tmp_str);
else if VG_STR_CLO( arg, "--cachegrind-out-file", clo_cachegrind_out_file) {}
else if VG_BOOL_CLO(arg, "--cache-sim", clo_cache_sim) {}
Modified: trunk/callgrind/sim.c
===================================================================
--- trunk/callgrind/sim.c 2010-07-05 07:21:22 UTC (rev 11208)
+++ trunk/callgrind/sim.c 2010-07-06 04:05:23 UTC (rev 11209)
@@ -1271,49 +1271,40 @@
static cache_t clo_L2_cache = UNDEFINED_CACHE;
-/* Checks cache config is ok; makes it so if not. */
-static
-void check_cache(cache_t* cache, Char *name)
+// Checks cache config is ok. Returns NULL if ok, or a pointer to an error
+// string otherwise.
+static Char* check_cache(cache_t* cache)
{
- /* Simulator requires line size and set count to be powers of two */
+ // Simulator requires line size and set count to be powers of two.
if (( cache->size % (cache->line_size * cache->assoc) != 0) ||
- (-1 == VG_(log2)(cache->size/cache->line_size/cache->assoc))) {
- VG_(message)(Vg_UserMsg,
- "error: %s set count not a power of two; aborting.\n",
- name);
+ (-1 == VG_(log2)(cache->size/cache->line_size/cache->assoc)))
+ {
+ return "Cache set count is not a power of two.\n";
}
+ // Simulator requires line size to be a power of two.
if (-1 == VG_(log2)(cache->line_size)) {
- VG_(message)(Vg_UserMsg,
- "error: %s line size of %dB not a power of two; aborting.\n",
- name, cache->line_size);
- VG_(exit)(1);
+ return "Cache line size is not a power of two.\n";
}
// Then check line size >= 16 -- any smaller and a single instruction could
// straddle three cache lines, which breaks a simulation assertion and is
// stupid anyway.
if (cache->line_size < MIN_LINE_SIZE) {
- VG_(message)(Vg_UserMsg,
- "error: %s line size of %dB too small; aborting.\n",
- name, cache->line_size);
- VG_(exit)(1);
+ return "Cache line size is too small.\n";
}
/* Then check cache size > line size (causes seg faults if not). */
if (cache->size <= cache->line_size) {
- VG_(message)(Vg_UserMsg,
- "error: %s cache size of %dB <= line size of %dB; aborting.\n",
- name, cache->size, cache->line_size);
- VG_(exit)(1);
+ return "Cache size <= line size.\n";
}
/* Then check assoc <= (size / line size) (seg faults otherwise). */
if (cache->assoc > (cache->size / cache->line_size)) {
- VG_(message)(Vg_UserMsg,
- "warning: %s associativity > (size / line size); aborting.\n", name);
- VG_(exit)(1);
+ return "Cache associativity > (size / line size).\n";
}
+
+ return NULL;
}
static
@@ -1321,27 +1312,27 @@
{
#define DEFINED(L) (-1 != L.size || -1 != L.assoc || -1 != L.line_size)
- Int n_clos = 0;
+ Char* checkRes;
- // Count how many were defined on the command line.
- if (DEFINED(clo_I1_cache)) { n_clos++; }
- if (DEFINED(clo_D1_cache)) { n_clos++; }
- if (DEFINED(clo_L2_cache)) { n_clos++; }
+ Bool all_caches_clo_defined =
+ (DEFINED(clo_I1_cache) &&
+ DEFINED(clo_D1_cache) &&
+ DEFINED(clo_L2_cache));
// Set the cache config (using auto-detection, if supported by the
- // architecture)
- VG_(configure_caches)( I1c, D1c, L2c, (3 == n_clos) );
+ // architecture).
+ VG_(configure_caches)( I1c, D1c, L2c, all_caches_clo_defined );
+ // Check the default/auto-detected values.
+ checkRes = check_cache(I1c); tl_assert(!checkRes);
+ checkRes = check_cache(D1c); tl_assert(!checkRes);
+ checkRes = check_cache(L2c); tl_assert(!checkRes);
+
// Then replace with any defined on the command line.
if (DEFINED(clo_I1_cache)) { *I1c = clo_I1_cache; }
if (DEFINED(clo_D1_cache)) { *D1c = clo_D1_cache; }
if (DEFINED(clo_L2_cache)) { *L2c = clo_L2_cache; }
- // Then check values and fix if not acceptable.
- check_cache(I1c, "I1");
- check_cache(D1c, "D1");
- check_cache(L2c, "L2");
-
if (VG_(clo_verbosity) > 1) {
VG_(message)(Vg_UserMsg, "Cache configuration used:\n");
VG_(message)(Vg_UserMsg, " I1: %dB, %d-way, %dB lines\n",
@@ -1503,13 +1494,14 @@
);
}
-static void parse_opt ( cache_t* cache, char* opt )
+static void parse_opt ( cache_t* cache, char* opt, Char* optval )
{
Long i1, i2, i3;
Char* endptr;
+ Char* checkRes;
// Option argument looks like "65536,2,64". Extract them.
- i1 = VG_(strtoll10)(opt, &endptr); if (*endptr != ',') goto bad;
+ i1 = VG_(strtoll10)(optval, &endptr); if (*endptr != ',') goto bad;
i2 = VG_(strtoll10)(endptr+1, &endptr); if (*endptr != ',') goto bad;
i3 = VG_(strtoll10)(endptr+1, &endptr); if (*endptr != '\0') goto bad;
@@ -1521,15 +1513,20 @@
if (cache->assoc != i2) goto overflow;
if (cache->line_size != i3) goto overflow;
+ checkRes = check_cache(cache);
+ if (checkRes) {
+ VG_(fmsg)("%s", checkRes);
+ goto bad;
+ }
+
return;
- overflow:
- VG_(message)(Vg_UserMsg,
- "one of the cache parameters was too large and overflowed\n");
bad:
- // XXX: this omits the "--I1/D1/L2=" part from the message, but that's
- // not a big deal.
- VG_(err_bad_option)(opt);
+ VG_(fmsg_bad_option)(opt, "");
+
+ overflow:
+ VG_(fmsg_bad_option)(opt,
+ "One of the cache parameters was too large and overflowed.\n");
}
/* Check for command line option for cache configuration.
@@ -1553,11 +1550,11 @@
}
else if VG_STR_CLO(arg, "--I1", tmp_str)
- parse_opt(&clo_I1_cache, tmp_str);
+ parse_opt(&clo_I1_cache, arg, tmp_str);
else if VG_STR_CLO(arg, "--D1", tmp_str)
- parse_opt(&clo_D1_cache, tmp_str);
+ parse_opt(&clo_D1_cache, arg, tmp_str);
else if VG_STR_CLO(arg, "--L2", tmp_str)
- parse_opt(&clo_L2_cache, tmp_str);
+ parse_opt(&clo_L2_cache, arg, tmp_str);
else
return False;
Modified: trunk/coregrind/m_libcprint.c
===================================================================
--- trunk/coregrind/m_libcprint.c 2010-07-05 07:21:22 UTC (rev 11208)
+++ trunk/coregrind/m_libcprint.c 2010-07-06 04:05:23 UTC (rev 11209)
@@ -400,14 +400,6 @@
HChar ch;
Int i, depth;
- switch (b->kind) {
- case Vg_UserMsg: ch = '='; break;
- case Vg_DebugMsg: ch = '-'; break;
- case Vg_DebugExtraMsg: ch = '+'; break;
- case Vg_ClientMsg: ch = '*'; break;
- default: ch = '?'; break;
- }
-
// Print one '>' in front of the messages for each level of
// self-hosting being performed.
depth = RUNNING_ON_VALGRIND;
@@ -417,26 +409,48 @@
b->buf[b->buf_used++] = '>';
}
- b->buf[b->buf_used++] = ch;
- b->buf[b->buf_used++] = ch;
+ if (Vg_FailMsg == b->kind) {
+ // "valgrind: " prefix.
+ b->buf[b->buf_used++] = 'v';
+ b->buf[b->buf_used++] = 'a';
+ b->buf[b->buf_used++] = 'l';
+ b->buf[b->buf_used++] = 'g';
+ b->buf[b->buf_used++] = 'r';
+ b->buf[b->buf_used++] = 'i';
+ b->buf[b->buf_used++] = 'n';
+ b->buf[b->buf_used++] = 'd';
+ b->buf[b->buf_used++] = ':';
+ b->buf[b->buf_used++] = ' ';
+ } else {
+ switch (b->kind) {
+ case Vg_UserMsg: ch = '='; break;
+ case Vg_DebugMsg: ch = '-'; break;
+ case Vg_DebugExtraMsg: ch = '+'; break;
+ case Vg_ClientMsg: ch = '*'; break;
+ default: ch = '?'; break;
+ }
- if (VG_(clo_time_stamp)) {
- VG_(memset)(tmp, 0, sizeof(tmp));
- VG_(elapsed_wallclock_time)(tmp);
+ b->buf[b->buf_used++] = ch;
+ b->buf[b->buf_used++] = ch;
+
+ if (VG_(clo_time_stamp)) {
+ VG_(memset)(tmp, 0, sizeof(tmp));
+ VG_(elapsed_wallclock_time)(tmp);
+ tmp[sizeof(tmp)-1] = 0;
+ for (i = 0; tmp[i]; i++)
+ b->buf[b->buf_used++] = tmp[i];
+ }
+
+ VG_(sprintf)(tmp, "%d", VG_(getpid)());
tmp[sizeof(tmp)-1] = 0;
for (i = 0; tmp[i]; i++)
b->buf[b->buf_used++] = tmp[i];
+
+ b->buf[b->buf_used++] = ch;
+ b->buf[b->buf_used++] = ch;
+ b->buf[b->buf_used++] = ' ';
}
- VG_(sprintf)(tmp, "%d", VG_(getpid)());
- tmp[sizeof(tmp)-1] = 0;
- for (i = 0; tmp[i]; i++)
- b->buf[b->buf_used++] = tmp[i];
-
- b->buf[b->buf_used++] = ch;
- b->buf[b->buf_used++] = ch;
- b->buf[b->buf_used++] = ' ';
-
/* We can't possibly have stuffed 96 chars in merely as a result
of making the preamble (can we?) */
vg_assert(b->buf_used < sizeof(b->buf)-32);
@@ -503,7 +517,36 @@
return count;
}
+static void revert_to_stderr ( void )
+{
+ VG_(log_output_sink).fd = 2; /* stderr */
+ VG_(log_output_sink).is_socket = False;
+}
+
/* VG_(message) variants with hardwired first argument. */
+
+UInt VG_(fmsg) ( const HChar* format, ... )
+{
+ UInt count;
+ va_list vargs;
+ va_start(vargs,format);
+ count = VG_(vmessage) ( Vg_FailMsg, format, vargs );
+ va_end(vargs);
+ return count;
+}
+
+void VG_(fmsg_bad_option) ( HChar* opt, const HChar* format, ... )
+{
+ va_list vargs;
+ va_start(vargs,format);
+ revert_to_stderr();
+ VG_(message) (Vg_FailMsg, "Bad option: %s\n", opt);
+ VG_(vmessage)(Vg_FailMsg, format, vargs );
+ VG_(message) (Vg_FailMsg, "Use --help for more information or consult the user manual.\n");
+ VG_(exit)(1);
+ va_end(vargs);
+}
+
UInt VG_(umsg) ( const HChar* format, ... )
{
UInt count;
@@ -543,7 +586,25 @@
b->buf_used = 0;
}
+__attribute__((noreturn))
+void VG_(err_missing_prog) ( void )
+{
+ revert_to_stderr();
+ VG_(fmsg)("no program specified\n");
+ VG_(fmsg)("Use --help for more information.\n");
+ VG_(exit)(1);
+}
+__attribute__((noreturn))
+void VG_(err_config_error) ( Char* msg )
+{
+ revert_to_stderr();
+ VG_(fmsg)("Startup or configuration error:\n %s\n", msg);
+ VG_(fmsg)("Unable to start up properly. Giving up.\n");
+ VG_(exit)(1);
+}
+
+
/*--------------------------------------------------------------------*/
/*--- end ---*/
/*--------------------------------------------------------------------*/
Modified: trunk/coregrind/m_main.c
===================================================================
--- trunk/coregrind/m_main.c 2010-07-05 07:21:22 UTC (rev 11208)
+++ trunk/coregrind/m_main.c 2010-07-06 04:05:23 UTC (rev 11209)
@@ -177,8 +177,8 @@
Char* usage2 =
"\n"
" debugging options for all Valgrind tools:\n"
-" --stats=no|yes show tool and core statistics [no]\n"
" -d show verbose debugging output\n"
+" --stats=no|yes show tool and core statistics [no]\n"
" --sanity-level=<number> level of sanity checking to do [1]\n"
" --trace-flags=<XXXXXXXX> show generated code? (X = 0|1) [00000000]\n"
" --profile-flags=<XXXXXXXX> ditto, but for profiling (X = 0|1) [00000000]\n"
@@ -542,11 +542,9 @@
else if VG_STR_CLO(arg, "--suppressions", tmp_str) {
if (VG_(clo_n_suppressions) >= VG_CLO_MAX_SFILES) {
- VG_(message)(Vg_UserMsg,
- "Too many suppression files specified.\n");
- VG_(message)(Vg_UserMsg,
- "Increase VG_CLO_MAX_SFILES and recompile.\n");
- VG_(err_bad_option)(arg);
+ VG_(fmsg_bad_option)(arg,
+ "Too many suppression files specified.\n"
+ "Increase VG_CLO_MAX_SFILES and recompile.\n");
}
VG_(clo_suppressions)[VG_(clo_n_suppressions)] = tmp_str;
VG_(clo_n_suppressions)++;
@@ -554,11 +552,9 @@
else if VG_STR_CLO(arg, "--require-text-symbol", tmp_str) {
if (VG_(clo_n_req_tsyms) >= VG_CLO_MAX_REQ_TSYMS) {
- VG_(message)(Vg_UserMsg,
- "Too many --require-text-symbol= specifications.\n");
- VG_(message)(Vg_UserMsg,
- "Increase VG_CLO_MAX_REQ_TSYMS and recompile.\n");
- VG_(err_bad_option)(arg);
+ VG_(fmsg_bad_option)(arg,
+ "Too many --require-text-symbol= specifications.\n"
+ "Increase VG_CLO_MAX_REQ_TSYMS and recompile.\n");
}
/* String needs to be of the form C?*C?*, where C is any
character, but is the same both times. Having it in this
@@ -577,9 +573,8 @@
ok = VG_(string_match)(patt, tmp_str);
}
if (!ok) {
- VG_(message)(Vg_UserMsg,
- "Invalid --require-text-symbol= specification.\n");
- VG_(err_bad_option)(arg);
+ VG_(fmsg_bad_option)(arg,
+ "Invalid --require-text-symbol= specification.\n");
}
VG_(clo_req_tsyms)[VG_(clo_n_req_tsyms)] = tmp_str;
VG_(clo_n_req_tsyms)++;
@@ -590,17 +585,15 @@
Int j;
if (8 != VG_(strlen)(tmp_str)) {
- VG_(message)(Vg_UserMsg,
- "--trace-flags argument must have 8 digits\n");
- VG_(err_bad_option)(arg);
+ VG_(fmsg_bad_option)(arg,
+ "--trace-flags argument must have 8 digits\n");
}
for (j = 0; j < 8; j++) {
if ('0' == tmp_str[j]) { /* do nothing */ }
else if ('1' == tmp_str[j]) VG_(clo_trace_flags) |= (1 << (7-j));
else {
- VG_(message)(Vg_UserMsg, "--trace-flags argument can only "
- "contain 0s and 1s\n");
- VG_(err_bad_option)(arg);
+ VG_(fmsg_bad_option)(arg,
+ "--trace-flags argument can only contain 0s and 1s\n");
}
}
}
@@ -610,17 +603,15 @@
Int j;
if (8 != VG_(strlen)(tmp_str)) {
- VG_(message)(Vg_UserMsg,
- "--profile-flags argument must have 8 digits\n");
- VG_(err_bad_option)(arg);
+ VG_(fmsg_bad_option)(arg,
+ "--profile-flags argument must have 8 digits\n");
}
for (j = 0; j < 8; j++) {
if ('0' == tmp_str[j]) { /* do nothing */ }
else if ('1' == tmp_str[j]) VG_(clo_profile_flags) |= (1 << (7-j));
else {
- VG_(message)(Vg_UserMsg, "--profile-flags argument can only "
- "contain 0s and 1s\n");
- VG_(err_bad_option)(arg);
+ VG_(fmsg_bad_option)(arg,
+ "--profile-flags argument can only contain 0s and 1s\n");
}
}
}
@@ -636,7 +627,7 @@
else if ( ! VG_(needs).command_line_options
|| ! VG_TDICT_CALL(tool_process_cmd_line_option, arg) ) {
- VG_(err_bad_option)(arg);
+ VG_(fmsg_bad_option)(arg, "");
}
}
@@ -659,20 +650,17 @@
if (VG_(clo_gen_suppressions) > 0 &&
!VG_(needs).core_errors && !VG_(needs).tool_errors) {
- VG_(message)(Vg_UserMsg,
- "Can't use --gen-suppressions= with this tool,\n");
- VG_(message)(Vg_UserMsg,
- "as it doesn't generate errors.\n");
- VG_(err_bad_option)("--gen-suppressions=");
+ VG_(fmsg_bad_option)("--gen-suppressions=yes",
+ "Can't use --gen-suppressions= with %s\n"
+ "because it doesn't generate errors.\n", VG_(details).name);
}
/* If XML output is requested, check that the tool actually
supports it. */
if (VG_(clo_xml) && !VG_(needs).xml_output) {
VG_(clo_xml) = False;
- VG_(message)(Vg_UserMsg,
+ VG_(fmsg_bad_option)("--xml=yes",
"%s does not support XML output.\n", VG_(details).name);
- VG_(err_bad_option)("--xml=yes");
/*NOTREACHED*/
}
@@ -693,33 +681,28 @@
(--gen-suppressions=all is still OK since we don't need any
user interaction in this case.) */
if (VG_(clo_gen_suppressions) == 1) {
- VG_(umsg)(
- "When --xml=yes is specified, only --gen-suppressions=no\n"
- "or --gen-suppressions=all are allowed, but not "
+ VG_(fmsg_bad_option)(
+ "--xml=yes together with --gen-suppressions=yes",
+ "When --xml=yes is specified, --gen-suppressions=no\n"
+ "or --gen-suppressions=all is allowed, but not "
"--gen-suppressions=yes.\n");
- /* FIXME: this is really a misuse of VG_(err_bad_option). */
- VG_(err_bad_option)(
- "--xml=yes together with --gen-suppressions=yes");
}
/* We can't allow DB attaching (or we maybe could, but results
could be chaotic ..) since it requires user input. Hence
disallow. */
if (VG_(clo_db_attach)) {
- VG_(umsg)("--db-attach=yes is not allowed in XML mode,\n"
- "as it would require user input.\n");
- /* FIXME: this is really a misuse of VG_(err_bad_option). */
- VG_(err_bad_option)(
- "--xml=yes together with --db-attach=yes");
+ VG_(fmsg_bad_option)(
+ "--xml=yes together with --db-attach=yes",
+ "--db-attach=yes is not allowed with --xml=yes\n"
+ "because it would require user input.\n");
}
/* Disallow dump_error in XML mode; sounds like a recipe for
chaos. No big deal; dump_error is a flag for debugging V
itself. */
if (VG_(clo_dump_error) > 0) {
- /* FIXME: this is really a misuse of VG_(err_bad_option). */
- VG_(err_bad_option)(
- "--xml=yes together with --dump-error=");
+ VG_(fmsg_bad_option)("--xml=yes together with --dump-error", "");
}
/* Disable error limits (this might be a bad idea!) */
@@ -777,11 +760,9 @@
tmp_log_fd = sr_Res(sres);
VG_(clo_log_fname_expanded) = logfilename;
} else {
- VG_(message)(Vg_UserMsg,
- "Can't create log file '%s' (%s); giving up!\n",
- logfilename, VG_(strerror)(sr_Err(sres)));
- VG_(err_bad_option)(
- "--log-file=<file> (didn't work out for some reason.)");
+ VG_(fmsg)("can't create log file '%s': %s\n",
+ logfilename, VG_(strerror)(sr_Err(sres)));
+ VG_(exit)(1);
/*NOTREACHED*/
}
break;
@@ -792,23 +773,16 @@
vg_assert(VG_(strlen)(log_fsname_unexpanded) <= 900); /* paranoia */
tmp_log_fd = VG_(connect_via_socket)( log_fsname_unexpanded );
if (tmp_log_fd == -1) {
- VG_(message)(Vg_UserMsg,
- "Invalid --log-socket=ipaddr or "
- "--log-socket=ipaddr:port spec\n");
- VG_(message)(Vg_UserMsg,
- "of '%s'; giving up!\n", log_fsname_unexpanded );
- VG_(err_bad_option)(
- "--log-socket=");
+ VG_(fmsg)("Invalid --log-socket spec of '%s'\n",
+ log_fsname_unexpanded);
+ VG_(exit)(1);
/*NOTREACHED*/
}
if (tmp_log_fd == -2) {
- VG_(message)(Vg_UserMsg,
- "valgrind: failed to connect to logging server '%s'.\n",
- log_fsname_unexpanded );
- VG_(message)(Vg_UserMsg,
- "Log messages will sent to stderr instead.\n" );
- VG_(message)(Vg_UserMsg,
- "\n" );
+ VG_(umsg)("failed to connect to logging server '%s'.\n"
+ "Log messages will sent to stderr instead.\n",
+ log_fsname_unexpanded );
+
/* We don't change anything here. */
vg_assert(VG_(log_output_sink).fd == 2);
tmp_log_fd = 2;
@@ -848,11 +822,9 @@
*xml_fname_unexpanded = VG_(strdup)( "main.mpclo.2",
xml_fsname_unexpanded );
} else {
- VG_(message)(Vg_UserMsg,
- "Can't create XML file '%s' (%s); giving up!\n",
- xmlfilename, VG_(strerror)(sr_Err(sres)));
- VG_(err_bad_option)(
- "--xml-file=<file> (didn't work out for some reason.)");
+ VG_(fmsg)("can't create XML file '%s': %s\n",
+ xmlfilename, VG_(strerror)(sr_Err(sres)));
+ VG_(exit)(1);
/*NOTREACHED*/
}
break;
@@ -863,23 +835,15 @@
vg_assert(VG_(strlen)(xml_fsname_unexpanded) <= 900); /* paranoia */
tmp_xml_fd = VG_(connect_via_socket)( xml_fsname_unexpanded );
if (tmp_xml_fd == -1) {
- VG_(message)(Vg_UserMsg,
- "Invalid --xml-socket=ipaddr or "
- "--xml-socket=ipaddr:port spec\n");
- VG_(message)(Vg_UserMsg,
- "of '%s'; giving up!\n", xml_fsname_unexpanded );
- VG_(err_bad_option)(
- "--xml-socket=");
+ VG_(fmsg)("Invalid --xml-socket spec of '%s'\n",
+ xml_fsname_unexpanded );
+ VG_(exit)(1);
/*NOTREACHED*/
}
if (tmp_xml_fd == -2) {
- VG_(message)(Vg_UserMsg,
- "valgrind: failed to connect to XML logging server '%s'.\n",
- xml_fsname_unexpanded );
- VG_(message)(Vg_UserMsg,
- "XML output will sent to stderr instead.\n" );
- VG_(message)(Vg_UserMsg,
- "\n" );
+ VG_(umsg)("failed to connect to XML logging server '%s'.\n"
+ "XML output will sent to stderr instead.\n",
+ xml_fsname_unexpanded);
/* We don't change anything here. */
vg_assert(VG_(xml_output_sink).fd == 2);
tmp_xml_fd = 2;
@@ -897,13 +861,12 @@
but that is likely to confuse the hell out of users, which is
distinctly Ungood. */
if (VG_(clo_xml) && tmp_xml_fd == -1) {
- VG_(umsg)(
+ VG_(fmsg_bad_option)(
+ "--xml=yes, but no XML destination specified",
"--xml=yes has been specified, but there is no XML output\n"
"destination. You must specify an XML output destination\n"
- "using --xml-fd=, --xml-file= or --xml=socket=.\n" );
- /* FIXME: this is really a misuse of VG_(err_bad_option). */
- VG_(err_bad_option)(
- "--xml=yes, but no XML destination specified");
+ "using --xml-fd, --xml-file or --xml-socket.\n"
+ );
}
// Finalise the output fds: the log fd ..
@@ -1133,8 +1096,8 @@
VG_(printf_xml_no_f_c)(" <exe>%t</exe>\n",
VG_(name_of_launcher));
else
- VG_(printf_xml_no_f_c)(Vg_UserMsg, " <exe>%t</exe>\n",
- "(launcher name unknown)");
+ VG_(printf_xml_no_f_c)(" <exe>%t</exe>\n",
+ "(launcher name unknown)");
for (i = 0; i < VG_(sizeXA)( VG_(args_for_valgrind) ); i++) {
VG_(printf_xml_no_f_c)(
" <arg>%t</arg>\n",
Modified: trunk/coregrind/m_options.c
===================================================================
--- trunk/coregrind/m_options.c 2010-07-05 07:21:22 UTC (rev 11208)
+++ trunk/coregrind/m_options.c 2010-07-06 04:05:23 UTC (rev 11209)
@@ -1,7 +1,6 @@
/*--------------------------------------------------------------------*/
-/*--- Command line options. ---*/
-/*--- m_options.c ---*/
+/*--- Command line options. m_options.c ---*/
/*--------------------------------------------------------------------*/
/*
@@ -100,42 +99,9 @@
/*====================================================================*/
-/*=== Command line errors ===*/
+/*=== File expansion ===*/
/*====================================================================*/
-static void revert_to_stderr ( void )
-{
- VG_(log_output_sink).fd = 2; /* stderr */
- VG_(log_output_sink).is_socket = False;
-}
-
-__attribute__((noreturn))
-void VG_(err_bad_option) ( Char* opt )
-{
- revert_to_stderr();
- VG_(printf)("valgrind: Bad option '%s'; aborting.\n", opt);
- VG_(printf)("valgrind: Use --help for more information.\n");
- VG_(exit)(1);
-}
-
-__attribute__((noreturn))
-void VG_(err_missing_prog) ( void )
-{
- revert_to_stderr();
- VG_(printf)("valgrind: no program specified\n");
- VG_(printf)("valgrind: Use --help for more information.\n");
- VG_(exit)(1);
-}
-
-__attribute__((noreturn))
-void VG_(err_config_error) ( Char* msg )
-{
- revert_to_stderr();
- VG_(printf)("valgrind: Startup or configuration error:\n %s\n", msg);
- VG_(printf)("valgrind: Unable to start up properly. Giving up.\n");
- VG_(exit)(1);
-}
-
// Copies the string, prepending it with the startup working directory, and
// expanding %p and %q entries. Returns a new, malloc'd string.
Char* VG_(expand_file_name)(Char* option_name, Char* format)
@@ -149,7 +115,7 @@
if (VG_STREQ(format, "")) {
// Empty name, bad.
- VG_(umsg)("%s: filename is empty", option_name);
+ VG_(fmsg)("%s: filename is empty", option_name);
goto bad;
}
@@ -158,11 +124,13 @@
// that we don't allow a legitimate filename beginning with '~' but that
// seems very unlikely.
if (format[0] == '~') {
- VG_(umsg)("%s: filename begins with '~'\n", option_name);
- VG_(umsg)("You probably expected the shell to expand the '~', but it\n");
- VG_(umsg)("didn't. The rules for '~'-expansion "
- "vary from shell to shell.\n");
- VG_(umsg)("You might have more luck using $HOME instead.\n");
+ VG_(fmsg)(
+ "%s: 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
+ );
goto bad;
}
@@ -216,8 +184,7 @@
qualname = &format[i];
while (True) {
if (0 == format[i]) {
- VG_(message)(Vg_UserMsg, "%s: malformed %%q specifier\n",
- option_name);
+ VG_(fmsg)("%s: malformed %%q specifier\n", option_name);
goto bad;
} else if ('}' == format[i]) {
// Temporarily replace the '}' with NUL to extract var
@@ -225,9 +192,8 @@
format[i] = 0;
qual = VG_(getenv)(qualname);
if (NULL == qual) {
- VG_(message)(Vg_UserMsg,
- "%s: environment variable %s is not set\n",
- option_name, qualname);
+ VG_(fmsg)("%s: environment variable %s is not set\n",
+ option_name, qualname);
format[i] = '}'; // Put the '}' back.
goto bad;
}
@@ -240,15 +206,14 @@
ENSURE_THIS_MUCH_SPACE(VG_(strlen)(qual));
j += VG_(sprintf)(&out[j], "%s", qual);
} else {
- VG_(message)(Vg_UserMsg,
- "%s: expected '{' after '%%q'\n", option_name);
+ VG_(fmsg)("%s: expected '{' after '%%q'\n", option_name);
goto bad;
}
}
else {
// Something else, abort.
- VG_(message)(Vg_UserMsg,
- "%s: expected 'p' or 'q' or '%%' after '%%'\n", option_name);
+ VG_(fmsg)("%s: expected 'p' or 'q' or '%%' after '%%'\n",
+ option_name);
goto bad;
}
}
@@ -265,7 +230,7 @@
VG_(strcpy)(opt, option_name);
VG_(strcat)(opt, "=");
VG_(strcat)(opt, format);
- VG_(err_bad_option)(opt);
+ VG_(fmsg_bad_option)(opt, "");
}
}
@@ -335,5 +300,5 @@
/*--------------------------------------------------------------------*/
-/*--- end m_options.c ---*/
+/*--- end ---*/
/*--------------------------------------------------------------------*/
Modified: trunk/coregrind/m_replacemalloc/replacemalloc_core.c
===================================================================
--- trunk/coregrind/m_replacemalloc/replacemalloc_core.c 2010-07-05 07:21:22 UTC (rev 11208)
+++ trunk/coregrind/m_replacemalloc/replacemalloc_core.c 2010-07-06 04:05:23 UTC (rev 11209)
@@ -58,12 +58,9 @@
VG_(clo_alignment) > 4096 ||
VG_(log2)( VG_(clo_alignment) ) == -1 /* not a power of 2 */)
{
- VG_(message)(Vg_UserMsg,
- "Invalid --alignment= setting. "
- "Should be a power of 2, >= %d, <= 4096.\n",
- VG_MIN_MALLOC_SZB
- );
- VG_(err_bad_option)("--alignment");
+ VG_(fmsg_bad_option)(arg,
+ "Alignment must be a power of 2 in the range %d..4096.\n",
+ VG_MIN_MALLOC_SZB);
}
}
Modified: trunk/coregrind/m_ume/main.c
===================================================================
--- trunk/coregrind/m_ume/main.c 2010-07-05 07:21:22 UTC (rev 11208)
+++ trunk/coregrind/m_ume/main.c 2010-07-06 04:05:23 UTC (rev 11209)
@@ -189,7 +189,7 @@
// Something went wrong. This will only happen if we earlier
// succeeded in opening the file but fail here (eg. the file was
// deleted between then and now).
- VG_(printf)("valgrind: %s: unknown error\n", f);
+ VG_(fmsg)("%s: unknown error\n", f);
VG_(exit)(126); // 126 == NOEXEC
}
}
@@ -210,7 +210,7 @@
// Is it a binary file?
if (is_binary_file(exe_name)) {
- VG_(printf)("valgrind: %s: cannot execute binary file\n", exe_name);
+ VG_(fmsg)("%s: cannot execute binary file\n", exe_name);
VG_(exit)(126); // 126 == NOEXEC
}
@@ -226,7 +226,7 @@
if (0 != ret) {
// Something went wrong with executing the default interpreter
- VG_(printf)("valgrind: %s: bad interpreter (%s): %s\n",
+ VG_(fmsg)("%s: bad interpreter (%s): %s\n",
exe_name, info->interp_name, VG_(strerror)(ret));
VG_(exit)(126); // 126 == NOEXEC
}
@@ -238,21 +238,20 @@
// Was it a directory?
res = VG_(stat)(exe_name, &st);
if (!sr_isError(res) && VKI_S_ISDIR(st.mode)) {
- VG_(printf)("valgrind: %s: is a directory\n", exe_name);
+ VG_(fmsg)("%s: is a directory\n", exe_name);
// Was it not executable?
} else if (0 != VG_(check_executable)(NULL, exe_name,
False/*allow_setuid*/)) {
- VG_(printf)("valgrind: %s: %s\n", exe_name, VG_(strerror)(ret));
+ VG_(fmsg)("%s: %s\n", exe_name, VG_(strerror)(ret));
// Did it start with "#!"? If so, it must have been a bad interpreter.
} else if (is_hash_bang_file(exe_name)) {
- VG_(printf)("valgrind: %s: bad interpreter: %s\n",
- exe_name, VG_(strerror)(ret));
+ VG_(fmsg)("%s: bad interpreter: %s\n", exe_name, VG_(strerror)(ret));
// Otherwise it was something else.
} else {
- VG_(printf)("valgrind: %s: %s\n", exe_name, VG_(strerror)(ret));
+ VG_(fmsg)("%s: %s\n", exe_name, VG_(strerror)(ret));
}
// 126 means NOEXEC; I think this is Posix, and that in some cases we
// should be returning 127, meaning NOTFOUND. Oh well.
Modified: trunk/coregrind/pub_core_libcprint.h
===================================================================
--- trunk/coregrind/pub_core_libcprint.h 2010-07-05 07:21:22 UTC (rev 11208)
+++ trunk/coregrind/pub_core_libcprint.h 2010-07-06 04:05:23 UTC (rev 11209)
@@ -54,6 +54,16 @@
m_main during startup. */
void VG_(elapsed_wallclock_time) ( /*OUT*/HChar* buf );
+/* Call this if the executable is missing. This function prints an
+ error message, then shuts down the entire system. */
+__attribute__((noreturn))
+extern void VG_(err_missing_prog) ( void );
+
+/* Similarly - complain and stop if there is some kind of config
+ error. */
+__attribute__((noreturn))
+extern void VG_(err_config_error) ( Char* msg );
+
#endif // __PUB_CORE_LIBCPRINT_H
/*--------------------------------------------------------------------*/
Modified: trunk/coregrind/pub_core_options.h
===================================================================
--- trunk/coregrind/pub_core_options.h 2010-07-05 07:21:22 UTC (rev 11208)
+++ trunk/coregrind/pub_core_options.h 2010-07-06 04:05:23 UTC (rev 11209)
@@ -210,18 +210,6 @@
.dSYM directories as necessary? */
extern Bool VG_(clo_dsymutil);
-/* --------- Functions --------- */
-
-/* Call this if the executable is missing. This function prints an
- error message, then shuts down the entire system. */
-__attribute__((noreturn))
-extern void VG_(err_missing_prog) ( void );
-
-/* Similarly - complain and stop if there is some kind of config
- error. */
-__attribute__((noreturn))
-extern void VG_(err_config_error) ( Char* msg );
-
/* Should we trace into this child executable (across execve etc) ?
This involves considering --trace-children=, --trace-children-skip=
and the name of the executable. */
Modified: trunk/include/pub_tool_libcprint.h
===================================================================
--- trunk/include/pub_tool_libcprint.h 2010-07-05 07:21:22 UTC (rev 11208)
+++ trunk/include/pub_tool_libcprint.h 2010-07-06 04:05:23 UTC (rev 11209)
@@ -32,19 +32,9 @@
#define __PUB_TOOL_LIBCPRINT_H
/* ---------------------------------------------------------------------
- Basic printing
+ Formatting functions
------------------------------------------------------------------ */
-/* Note that they all output to the file descriptor given by the
- --log-fd/--log-file/--log-socket argument, which defaults to 2
- (stderr). Hence no need for VG_(fprintf)().
-*/
-extern UInt VG_(printf) ( const HChar *format, ... )
- PRINTF_CHECK(1, 2);
-
-extern UInt VG_(vprintf) ( const HChar *format, va_list vargs )
- PRINTF_CHECK(1, 0);
-
extern UInt VG_(sprintf) ( Char* buf, const HChar* format, ... )
PRINTF_CHECK(2, 3);
@@ -59,64 +49,97 @@
const HChar *format, va_list vargs )
PRINTF_CHECK(3, 0);
-/* Yet another, totally general, version of vprintf, which hands all
- output bytes to CHAR_SINK, passing it OPAQUE as the second arg. */
-extern void VG_(vcbprintf)( void(*char_sink)(HChar, void* opaque),
- void* opaque,
- const HChar* format, va_list vargs );
-
-/* These are the same as the non "_xml" versions above, except the
- output goes on the selected XML output channel instead of the
- normal one.
-*/
-extern UInt VG_(printf_xml) ( const HChar *format, ... )
- PRINTF_CHECK(1, 2);
-
-extern UInt VG_(vprintf_xml) ( const HChar *format, va_list vargs )
- PRINTF_CHECK(1, 0);
-
-extern UInt VG_(printf_xml_no_f_c) ( const HChar *format, ... );
-
// Percentify n/m with d decimal places. Includes the '%' symbol at the end.
// Right justifies in 'buf'.
extern void VG_(percentify)(ULong n, ULong m, UInt d, Int n_buf, char buf[]);
/* ---------------------------------------------------------------------
- Messages for the user
+ Output-printing functions
------------------------------------------------------------------ */
+// Note that almost all output goes to the file descriptor given by the
+// --log-fd/--log-file/--log-socket argument, which defaults to 2 (stderr).
+// (Except that some text always goes to stdout/stderr at startup, and
+// debugging messages always go to stderr.) Hence no need for
+// VG_(fprintf)().
+
/* No, really. I _am_ that strange. */
#define OINK(nnn) VG_(message)(Vg_DebugMsg, "OINK %d\n",nnn)
-/* Print a message prefixed by "??<pid>?? "; '?' depends on the VgMsgKind.
+/* Print a message with a prefix that depends on the VgMsgKind.
Should be used for all user output. */
typedef
- enum { Vg_UserMsg, /* '?' == '=' */
- Vg_DebugMsg, /* '?' == '-' */
- Vg_DebugExtraMsg, /* '?' == '+' */
- Vg_ClientMsg /* '?' == '*' */
+ enum { // Prefix
+ Vg_FailMsg, // "valgrind:"
+ Vg_UserMsg, // "==pid=="
+ Vg_DebugMsg, // "--pid--"
+ Vg_DebugExtraMsg, // "++pid++"
+ Vg_ClientMsg // "**pid**"
}
VgMsgKind;
-/* Send a single-part message. The format specification may contain
- any ISO C format specifier or %t. No attempt is made to let the
- compiler verify consistency of the format string and the argument
- list. */
+// These print output that isn't prefixed with anything, and should be
+// used in very few cases, such as printing usage messages.
+extern UInt VG_(printf) ( const HChar *format, ... )
+ PRINTF_CHECK(1, 2);
+extern UInt VG_(vprintf) ( const HChar *format, va_list vargs )
+ PRINTF_CHECK(1, 0);
+
+// The "_no_f_c" functions here are just like their non-"_no_f_c" counterparts
+// but without the PRINTF_CHECK, so they can be used with our non-standard %t
+// format specifier.
+
+// These are the same as the non "_xml" versions above, except the
+// output goes on the selected XML output channel instead of the
+// normal one.
+extern UInt VG_(printf_xml) ( const HChar *format, ... )
+ PRINTF_CHECK(1, 2);
+
+extern UInt VG_(vprintf_xml) ( const HChar *format, va_list vargs )
+ PRINTF_CHECK(1, 0);
+
+extern UInt VG_(printf_xml_no_f_c) ( const HChar *format, ... );
+
+/* Yet another, totally general, version of vprintf, which hands all
+ output bytes to CHAR_SINK, passing it OPAQUE as the second arg. */
+extern void VG_(vcbprintf)( void(*char_sink)(HChar, void* opaque),
+ void* opaque,
+ const HChar* format, va_list vargs );
+
extern UInt VG_(message_no_f_c)( VgMsgKind kind, const HChar* format, ... );
-/* Send a single-part message. The format specification may contain
- any ISO C format specifier. The gcc compiler will verify
- consistency of the format string and the argument list. */
extern UInt VG_(message)( VgMsgKind kind, const HChar* format, ... )
- PRINTF_CHECK(2, 3);
+ PRINTF_CHECK(2, 3);
extern UInt VG_(vmessage)( VgMsgKind kind, const HChar* format, va_list vargs )
- PRINTF_CHECK(2, 0);
+ PRINTF_CHECK(2, 0);
// Short-cuts for VG_(message)().
+
+// This is used for messages printed due to start-up failures that occur
+// before the preamble is printed, eg. due a bad executable.
+extern UInt VG_(fmsg)( const HChar* format, ... ) PRINTF_CHECK(1, 2);
+
+// This is used if an option was bad for some reason. Note: don't use it just
+// because an option was unrecognised -- return 'False' from
+// VG_(tdict).tool_process_cmd_line_option) to indicate that -- use it if eg.
+// an option was given an inappropriate argument. This function prints an
+// error message, then shuts down the entire system.
+__attribute__((noreturn))
+extern void VG_(fmsg_bad_option) ( HChar* opt, const HChar* format, ... )
+ PRINTF_CHECK(2, 3);
+
+// This is used for messages that are interesting to the user: info about
+// their program (eg. preamble, tool error messages, postamble) or stuff they
+// requested.
extern UInt VG_(umsg)( const HChar* format, ... ) PRINTF_CHECK(1, 2);
+
+// This is used for debugging messages that are only of use to developers.
extern UInt VG_(dmsg)( const HChar* format, ... ) PRINTF_CHECK(1, 2);
+
+// This is used for additional debugging messages that are only of use to
+// developers.
extern UInt VG_(emsg)( const HChar* format, ... ) PRINTF_CHECK(1, 2);
/* Flush any output cached by previous calls to VG_(message) et al. */
Modified: trunk/include/pub_tool_options.h
===================================================================
--- trunk/include/pub_tool_options.h 2010-07-05 07:21:22 UTC (rev 11208)
+++ trunk/include/pub_tool_options.h 2010-07-06 04:05:23 UTC (rev 11209)
@@ -77,7 +77,7 @@
Long n = VG_(strtoll10)( val, &s ); \
(qq_var) = n; \
/* Check for non-numeralness, or overflow. */ \
- if ('\0' != s[0] || (qq_var) != n) VG_(err_bad_option)(qq_arg); \
+ if ('\0' != s[0] || (qq_var) != n) VG_(fmsg_bad_option)(qq_arg, ""); \
True; \
}) \
)
@@ -91,15 +91,16 @@
Char* s; \
Long n = VG_(strtoll##qq_base)( val, &s ); \
(qq_var) = n; \
+ /* MMM: separate the two cases, and explain the problem; likewise */ \
+ /* for all the other macros in this file. */ \
/* Check for non-numeralness, or overflow. */ \
/* Nb: it will overflow if qq_var is unsigned and qq_val is negative! */ \
- if ('\0' != s[0] || (qq_var) != n) VG_(err_bad_option)(qq_arg); \
+ if ('\0' != s[0] || (qq_var) != n) VG_(fmsg_bad_option)(qq_arg, ""); \
/* Check bounds. */ \
if ((qq_var) < (qq_lo) || (qq_var) > (qq_hi)) { \
- VG_(message)(Vg_UserMsg, \
- "'%s' argument must be between %lld and %lld\n", \
- (qq_option), (Long)(qq_lo), (Long)(qq_hi)); \
- VG_(err_bad_option)(qq_arg); \
+ VG_(fmsg_bad_option)(qq_arg, \
+ "'%s' argument must be between %lld and %lld\n", \
+ (qq_option), (Long)(qq_lo), (Long)(qq_hi)); \
} \
True; \
}) \
@@ -124,7 +125,7 @@
double n = VG_(strtod)( val, &s ); \
(qq_var) = n; \
/* Check for non-numeralness */ \
- if ('\0' != s[0]) VG_(err_bad_option)(qq_arg); \
+ if ('\0' != s[0]) VG_(fmsg_bad_option)(qq_arg, ""); \
True; \
}) \
)
@@ -165,15 +166,6 @@
extern Bool VG_(clo_show_below_main);
-/* Call this if a recognised option was bad for some reason. Note:
- don't use it just because an option was unrecognised -- return
- 'False' from VG_(tdict).tool_process_cmd_line_option) to indicate that --
- use it if eg. an option was given an inappropriate argument.
- This function prints an error message, then shuts down the entire system.
- It returns a Bool so it can be used in the _CLO_ macros. */
-__attribute__((noreturn))
-extern void VG_(err_bad_option) ( Char* opt );
-
/* Used to expand file names. "option_name" is the option name, eg.
"--log-file". 'format' is what follows, eg. "cachegrind.out.%p". In
'format':
Modified: trunk/include/pub_tool_tooliface.h
===================================================================
--- trunk/include/pub_tool_tooliface.h 2010-07-05 07:21:22 UTC (rev 11208)
+++ trunk/include/pub_tool_tooliface.h 2010-07-06 04:05:23 UTC (rev 11209)
@@ -374,10 +374,17 @@
/* Tool defines its own command line options? */
extern void VG_(needs_command_line_options) (
- // Return True if option was recognised. Presumably sets some state to
- // record the option as well. Nb: tools can assume that the argv will
- // never disappear. So they can, for example, store a pointer to a string
- // within an option, rather than having to make a copy.
+ // Return True if option was recognised, False if it wasn't (but also see
+ // below). Presumably sets some state to record the option as well.
+ //
+ // Nb: tools can assume that the argv will never disappear. So they can,
+ // for example, store a pointer to a string within an option, rather than
+ // having to make a copy.
+ //
+ // Options (and combinations of options) should be checked in this function
+ // if possible rather than in post_clo_init(), and if they are bad then
+ // VG_(fmsg_bad_option)() should be called. This ensures that the
+ // messaging is consistent with command line option errors from the core.
Bool (*process_cmd_line_option)(Char* argv),
// Print out command line usage for options for normal tool operation.
Modified: trunk/massif/ms_main.c
===================================================================
--- trunk/massif/ms_main.c 2010-07-05 07:21:22 UTC (rev 11208)
+++ trunk/massif/ms_main.c 2010-07-06 04:05:23 UTC (rev 11209)
@@ -442,7 +442,12 @@
VG_(addToXA)(ignore_fns, &tmp_str);
}
- else if VG_DBL_CLO(arg, "--threshold", clo_threshold) {}
+ else if VG_DBL_CLO(arg, "--threshold", clo_threshold) {
+ if (clo_threshold < 0 || clo_threshold > 100) {
+ VG_(fmsg_bad_option)(arg,
+ "--threshold must be between 0.0 and 100.0\n");
+ }
+ }
else if VG_DBL_CLO(arg, "--peak-inaccuracy", clo_peak_inaccuracy) {}
@@ -2386,14 +2391,10 @@
Char* s2;
// Check options.
- if (clo_threshold < 0 || clo_threshold > 100) {
- VG_(umsg)("--threshold must be between 0.0 and 100.0\n");
- VG_(err_bad_option)("--threshold");
- }
if (clo_pages_as_heap) {
if (clo_stacks) {
- VG_(umsg)("--pages-as-heap=yes cannot be used with --stacks=yes\n");
- VG_(err_bad_option)("--pages-as-heap=yes with --stacks=yes");
+ VG_(fmsg_bad_option)(
+ "--pages-as-heap=yes together with --stacks=yes", "");
}
}
if (!clo_heap) {
Modified: trunk/memcheck/mc_main.c
===================================================================
--- trunk/memcheck/mc_main.c 2010-07-05 07:21:22 UTC (rev 11208)
+++ trunk/memcheck/mc_main.c 2010-07-06 04:05:23 UTC (rev 11209)
@@ -4752,8 +4752,6 @@
static Bool mc_process_cmd_line_options(Char* arg)
{
Char* tmp_str;
- Char* bad_level_msg =
- "ERROR: --track-origins=yes has no effect when --undef-value-errors=no";
tl_assert( MC_(clo_mc_level) >= 1 && MC_(clo_mc_level) <= 3 );
@@ -4768,8 +4766,7 @@
*/
if (0 == VG_(strcmp)(arg, "--undef-value-errors=no")) {
if (MC_(clo_mc_level) == 3) {
- VG_(message)(Vg_DebugMsg, "%s\n", bad_level_msg);
- return False;
+ goto bad_level;
} else {
MC_(clo_mc_level) = 1;
return True;
@@ -4787,8 +4784,7 @@
}
if (0 == VG_(strcmp)(arg, "--track-origins=yes")) {
if (MC_(clo_mc_level) == 1) {
- VG_(message)(Vg_DebugMsg, "%s\n", bad_level_msg);
- return False;
+ goto bad_level;
} else {
MC_(clo_mc_level) = 3;
return True;
@@ -4854,6 +4850,11 @@
return VG_(replacement_malloc_process_cmd_line_option)(arg);
return True;
+
+
+ bad_level:
+ VG_(fmsg_bad_option)(arg,
+ "--track-origins=yes has no effect when --undef-value-errors=no.\n");
}
static void mc_print_usage(void)
Modified: trunk/none/tests/cmdline2.stdout.exp
===================================================================
--- trunk/none/tests/cmdline2.stdout.exp 2010-07-05 07:21:22 UTC (rev 11208)
+++ trunk/none/tests/cmdline2.stdout.exp 2010-07-06 04:05:23 UTC (rev 11209)
@@ -65,8 +65,8 @@
(none)
debugging options for all Valgrind tools:
+ -d show verbose debugging output
--stats=no|yes show tool and core statistics [no]
- -d show verbose debugging output
--sanity-level=<number> level of sanity checking to do [1]
--trace-flags=<XXXXXXXX> show generated code? (X = 0|1) [00000000]
--profile-flags=<XXXXXXXX> ditto, but for profiling (X = 0|1) [00000000]
Modified: trunk/none/tests/cmdline4.stderr.exp
===================================================================
--- trunk/none/tests/cmdline4.stderr.exp 2010-07-05 07:21:22 UTC (rev 11208)
+++ trunk/none/tests/cmdline4.stderr.exp 2010-07-06 04:05:23 UTC (rev 11209)
@@ -1,2 +1,2 @@
-valgrind: Bad option '--bad-bad-option'; aborting.
-valgrind: Use --help for more information.
+valgrind: Bad option: --bad-bad-option
+valgrind: Use --help for more information or consult the user manual.
|
