From 829fbc977d53f57def238d4f5da4fb28b8017e8e Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 12 Aug 2009 00:14:16 +0000 Subject: [PATCH] Output tweaks: - Always print a blank line after significant messages (eg. errors). This makes the handling of blank lines much simpler. - Don't print full stops at the end of messages. We mostly don't do it, so I got rid of all the remaining ones I could find for consistency. - Use --leak-check=full rather than --leak-check=yes, for consistency with docs and other messages. - Update partiallydefinedeq.stderr.exp2 for older changes. This commit only updates the code. Test updates will follow shortly. (I'm separating them so the code changes aren't swamped by the test changes in the SVN logs.) git-svn-id: svn://svn.valgrind.org/valgrind/trunk@10783 --- coregrind/m_errormgr.c | 26 +++----------------------- coregrind/m_main.c | 21 +++++++++++---------- coregrind/m_signals.c | 2 +- memcheck/mc_errors.c | 1 - memcheck/mc_leakcheck.c | 25 +++++++++++++------------ memcheck/mc_main.c | 17 +++++++---------- memcheck/mc_malloc_wrappers.c | 12 ++++-------- 7 files changed, 39 insertions(+), 65 deletions(-) diff --git a/coregrind/m_errormgr.c b/coregrind/m_errormgr.c index b026df4a56..48ecc20712 100644 --- a/coregrind/m_errormgr.c +++ b/coregrind/m_errormgr.c @@ -512,6 +512,7 @@ static void pp_Error ( Error* err, Bool allow_db_attach ) /* postamble */ VG_(printf_xml)("\n"); + VG_(printf_xml)("\n"); } else { @@ -524,9 +525,9 @@ static void pp_Error ( Error* err, Bool allow_db_attach ) } VG_TDICT_CALL( tool_pp_Error, err ); + VG_(umsg)("\n"); do_actions_on_error(err, allow_db_attach); - } } @@ -564,10 +565,6 @@ void construct_error ( Error* err, ThreadId tid, ErrorKind ekind, Addr a, -/* Shared between VG_(maybe_record_error)() and VG_(unique_error)(), - just for pretty printing purposes. */ -static Bool is_first_shown_context = True; - static Int n_errs_shown = 0; /* Top-level entry point to the error management subsystem. @@ -721,18 +718,9 @@ void VG_(maybe_record_error) ( ThreadId tid, if (p->supp == NULL) { n_err_contexts++; n_errs_found++; - /* A bit of prettyprinting, to ensure there's a blank line - in between each error. */ - if (!is_first_shown_context) { - if (VG_(clo_xml)) - VG_(printf_xml)("\n"); - else - VG_(umsg)("\n"); - } /* Actually show the error; more complex than you might think. */ pp_Error( p, /*allow_db_attach*/True ); /* update stats */ - is_first_shown_context = False; n_errs_shown++; } else { n_supp_contexts++; @@ -775,18 +763,9 @@ Bool VG_(unique_error) ( ThreadId tid, ErrorKind ekind, Addr a, Char* s, } if (print_error) { - /* A bit of prettyprinting, to ensure there's a blank line - in between each error. */ - if (!is_first_shown_context) { - if (VG_(clo_xml)) - VG_(printf_xml)("\n"); - else - VG_(umsg)("\n"); - } /* Actually show the error; more complex than you might think. */ pp_Error(&err, allow_db_attach); /* update stats */ - is_first_shown_context = False; n_errs_shown++; } return False; @@ -930,6 +909,7 @@ void VG_(show_error_counts_as_XML) ( void ) VG_(printf_xml)(" \n"); } VG_(printf_xml)("\n"); + VG_(printf_xml)("\n"); } diff --git a/coregrind/m_main.c b/coregrind/m_main.c index 75c6944ca7..60b8d7530f 100644 --- a/coregrind/m_main.c +++ b/coregrind/m_main.c @@ -1018,7 +1018,7 @@ static void print_preamble ( Bool logging_to_fd, VG_(printf_xml)("\n"); /* Tool details */ - umsg_or_xml( "%s%s%s%s, %s.%s\n", + umsg_or_xml( "%s%s%s%s, %s%s\n", xpre, VG_(details).name, NULL == VG_(details).version ? "" : "-", @@ -1029,7 +1029,7 @@ static void print_preamble ( Bool logging_to_fd, if (VG_(strlen)(toolname) >= 4 && VG_STREQN(4, toolname, "exp-")) { umsg_or_xml( - "%sNOTE: This is an Experimental-Class Valgrind Tool.%s\n", + "%sNOTE: This is an Experimental-Class Valgrind Tool%s\n", xpre, xpost ); } @@ -1060,6 +1060,7 @@ static void print_preamble ( Bool logging_to_fd, VG_(printf_xml)("\n"); } + // Print the parent PID, and other stuff, if necessary. if (!VG_(clo_xml) && VG_(clo_verbosity) > 0 && !logging_to_fd) { VG_(umsg)("Parent PID: %d\n", VG_(getppid)()); } @@ -1111,11 +1112,11 @@ static void print_preamble ( Bool logging_to_fd, VG_(printf_xml)("\n"); } - // Empty line after the preamble - if (VG_(clo_verbosity) > 0) - VG_(umsg)("\n"); + // Last thing in the preamble is a blank line. if (VG_(clo_xml)) VG_(printf_xml)("\n"); + else if (VG_(clo_verbosity) > 0) + VG_(umsg)("\n"); if (VG_(clo_verbosity) > 1) { SysRes fd; @@ -2394,10 +2395,11 @@ void shutdown_actions_NORETURN( ThreadId tid, // Finalisation: cleanup, messages, etc. Order not so important, only // affects what order the messages come. //-------------------------------------------------------------- - if (VG_(clo_verbosity) > 0) - VG_(message)(Vg_UserMsg, "\n"); + // First thing in the post-amble is a blank line. if (VG_(clo_xml)) VG_(printf_xml)("\n"); + else if (VG_(clo_verbosity) > 0) + VG_(message)(Vg_UserMsg, "\n"); if (VG_(clo_xml)) { HChar buf[50]; @@ -2405,7 +2407,8 @@ void shutdown_actions_NORETURN( ThreadId tid, VG_(printf_xml_no_f_c)( "\n" " FINISHED\n" " \n" - "\n", + "\n" + "\n", buf); } @@ -2420,9 +2423,7 @@ void shutdown_actions_NORETURN( ThreadId tid, /* Show the error counts. */ if (VG_(needs).core_errors || VG_(needs).tool_errors) { - VG_(printf_xml)( "\n" ); VG_(show_error_counts_as_XML)(); - VG_(printf_xml)( "\n" ); } /* In XML mode, this merely prints the used suppressions. */ diff --git a/coregrind/m_signals.c b/coregrind/m_signals.c index 8765c769e5..08f34070d7 100644 --- a/coregrind/m_signals.c +++ b/coregrind/m_signals.c @@ -1492,8 +1492,8 @@ static void default_action(const vki_siginfo_t *info, ThreadId tid) (could_core && is_signal_from_kernel(tid, sigNo, info->si_code)) ) && !VG_(clo_xml) ) { - VG_(umsg)("\n"); VG_(umsg)( + "\n" "Process terminating with default action of signal %d (%s)%s\n", sigNo, signame(sigNo), core ? ": dumping core" : ""); diff --git a/memcheck/mc_errors.c b/memcheck/mc_errors.c index 07b42742da..441c991d9c 100644 --- a/memcheck/mc_errors.c +++ b/memcheck/mc_errors.c @@ -729,7 +729,6 @@ void MC_(pp_Error) ( Error* err ) } VG_(pp_ExeContext)(lr->key.allocated_at); } else { /* ! if (xml) */ - emit("\n"); if (lr->indirect_szB > 0) { emit( "%'lu (%'lu direct, %'lu indirect) bytes in %'u blocks" diff --git a/memcheck/mc_leakcheck.c b/memcheck/mc_leakcheck.c index 0b8ea013f4..ce9a6e992c 100644 --- a/memcheck/mc_leakcheck.c +++ b/memcheck/mc_leakcheck.c @@ -879,23 +879,22 @@ static void print_results(ThreadId tid, Bool is_full_check) } if (VG_(clo_verbosity) > 0 && !VG_(clo_xml)) { - VG_(umsg)("\n"); VG_(umsg)("LEAK SUMMARY:\n"); - VG_(umsg)(" definitely lost: %'lu bytes in %'lu blocks.\n", + VG_(umsg)(" definitely lost: %'lu bytes in %'lu blocks\n", MC_(bytes_leaked), MC_(blocks_leaked) ); - VG_(umsg)(" indirectly lost: %'lu bytes in %'lu blocks.\n", + VG_(umsg)(" indirectly lost: %'lu bytes in %'lu blocks\n", MC_(bytes_indirect), MC_(blocks_indirect) ); - VG_(umsg)(" possibly lost: %'lu bytes in %'lu blocks.\n", + VG_(umsg)(" possibly lost: %'lu bytes in %'lu blocks\n", MC_(bytes_dubious), MC_(blocks_dubious) ); - VG_(umsg)(" still reachable: %'lu bytes in %'lu blocks.\n", + VG_(umsg)(" still reachable: %'lu bytes in %'lu blocks\n", MC_(bytes_reachable), MC_(blocks_reachable) ); - VG_(umsg)(" suppressed: %'lu bytes in %'lu blocks.\n", + VG_(umsg)(" suppressed: %'lu bytes in %'lu blocks\n", MC_(bytes_suppressed), MC_(blocks_suppressed) ); if (!is_full_check && (MC_(blocks_leaked) + MC_(blocks_indirect) + MC_(blocks_dubious) + MC_(blocks_reachable)) > 0) { VG_(umsg)("Rerun with --leak-check=full to see details " - "of leaked memory.\n"); + "of leaked memory\n"); } if (is_full_check && MC_(blocks_reachable) > 0 && !MC_(clo_show_reachable)) @@ -905,6 +904,7 @@ static void print_results(ThreadId tid, Bool is_full_check) VG_(umsg)("To see them, rerun with: --leak-check=full " "--show-reachable=yes\n"); } + VG_(umsg)("\n"); } } @@ -923,8 +923,8 @@ void MC_(detect_memory_leaks) ( ThreadId tid, LeakCheckMode mode ) if (lc_n_chunks == 0) { tl_assert(lc_chunks == NULL); if (VG_(clo_verbosity) >= 1 && !VG_(clo_xml)) { + VG_(umsg)("All heap blocks were freed -- no leaks are possible\n"); VG_(umsg)("\n"); - VG_(umsg)("All heap blocks were freed -- no leaks are possible.\n"); } return; } @@ -1012,8 +1012,7 @@ void MC_(detect_memory_leaks) ( ThreadId tid, LeakCheckMode mode ) // Verbosity. if (VG_(clo_verbosity) > 1 && !VG_(clo_xml)) { - VG_(umsg)( "\n" ); - VG_(umsg)( "Searching for pointers to %'d not-freed blocks.\n", + VG_(umsg)( "Searching for pointers to %'d not-freed blocks\n", lc_n_chunks ); } @@ -1073,8 +1072,10 @@ void MC_(detect_memory_leaks) ( ThreadId tid, LeakCheckMode mode ) // from the root-set has been traced. lc_process_markstack(/*clique*/-1); - if (VG_(clo_verbosity) > 1 && !VG_(clo_xml)) - VG_(umsg)("Checked %'lu bytes.\n", lc_scanned_szB); + if (VG_(clo_verbosity) > 1 && !VG_(clo_xml)) { + VG_(umsg)("Checked %'lu bytes\n", lc_scanned_szB); + VG_(umsg)( "\n" ); + } // Trace all the leaked blocks to determine which are directly leaked and // which are indirectly leaked. For each Unreached block, push it onto diff --git a/memcheck/mc_main.c b/memcheck/mc_main.c index 98043d5107..ffe75917f2 100644 --- a/memcheck/mc_main.c +++ b/memcheck/mc_main.c @@ -5571,16 +5571,13 @@ static void mc_fini ( Int exitcode ) if (MC_(clo_leak_check) != LC_Off) { MC_(detect_memory_leaks)(1/*bogus ThreadId*/, MC_(clo_leak_check)); - } - - if (VG_(clo_verbosity) == 1 && !VG_(clo_xml) - && MC_(clo_leak_check) == LC_Off) { - VG_(message)(Vg_UserMsg, - "For a detailed leak analysis, rerun with: --leak-check=yes\n"); - } - - if (VG_(clo_verbosity) >= 1 && !VG_(clo_xml)) { - VG_(message)(Vg_UserMsg, "\n"); + } else { + if (VG_(clo_verbosity) == 1 && !VG_(clo_xml)) { + VG_(umsg)( + "For a detailed leak analysis, rerun with: --leak-check=full\n" + "\n" + ); + } } if (VG_(clo_verbosity) == 1 && !VG_(clo_xml)) { diff --git a/memcheck/mc_malloc_wrappers.c b/memcheck/mc_malloc_wrappers.c index bef9f5195b..5d163231ee 100644 --- a/memcheck/mc_malloc_wrappers.c +++ b/memcheck/mc_malloc_wrappers.c @@ -904,14 +904,10 @@ void MC_(print_malloc_stats) ( void ) VG_(umsg)( "HEAP SUMMARY:\n" - ); - VG_(umsg)( - " in use at exit: %'llu bytes in %'lu blocks.\n", - nbytes, nblocks - ); - VG_(umsg)( - " total heap usage: %'lu allocs, %'lu frees, " - "%'llu bytes allocated.\n", + " in use at exit: %'llu bytes in %'lu blocks\n" + " total heap usage: %'lu allocs, %'lu frees, %'llu bytes allocated\n" + "\n", + nbytes, nblocks, cmalloc_n_mallocs, cmalloc_n_frees, cmalloc_bs_mallocd ); -- 2.47.2