|
From: Nicholas N. <n.n...@gm...> - 2009-08-03 23:04:23
|
On Tue, Aug 4, 2009 at 8:11 AM, Julian Seward<js...@ac...> wrote: > > As part of trying to fix > https://bugs.kde.org/show_bug.cgi?id=186790 properly, I made a > change which is long overdue: removing the printing of statistics > from "-v". Up to now, -v has caused the printing of some > additional information which might be useful to the user, such as > error and suppression counts, details of the CPU, and > redirections happening. But it also printed large amounts of > statistics, both for the core and the tool, which is mostly not > of interest to users, and is distracting. > > The attached patch removes statistics printing from -v, and gives > it a new flag --stats=no|yes [no]. For most tools this > significantly tidies up the -v output, and means we can continue > to restrict it to output which is of interest to users. > > I think this would be a good cleanup to have, but it changes the > flag behaviour a bit, so feedback would be good. * You'll need to update the user manual and NEWS file. In the NEWS file, you should mention both --stats and the change in normal output. * Memcheck output now looks like this: ==6281== ==6281== HEAP SUMMARY: ==6281== in use at exit: 15,090 bytes in 17 blocks. ==6281== heap usage: 17 allocs, 0 frees, 15,090 bytes allocated. ==6281== ==6281== Searching for pointers to 17 not-freed blocks. ==6281== Checked 1,123,768 bytes. ==6281== ==6281== LEAK SUMMARY: ==6281== definitely lost: 0 bytes in 0 blocks. ==6281== indirectly lost: 0 bytes in 0 blocks. ==6281== possibly lost: 0 bytes in 0 blocks. ==6281== still reachable: 10,694 bytes in 9 blocks. ==6281== suppressed: 4,396 bytes in 8 blocks. ==6281== Rerun with --leak-check=full to see details of leaked memory. ==6281== ==6281== For counts of detected errors, rerun with: -v ==6281== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 3 from 3) * Separating the HEAP SUMMARY is good, as previously it was mixed in with error and leak info. But maybe it could be more compact? The increasing indentation is so the ':'s line up, but it looks a bit weird. Previously it was: ==6284== malloc/free: in use at exit: 15,090 bytes in 17 blocks. ==6284== malloc/free: 17 allocs, 0 frees, 15,090 bytes allocated. But I'm not sure how to change it. But "total heap usage" might be clearer. * I think the "For counts of detected errors..." should come after "ERROR SUMMARY". * I think the "Searching for... Checked..." lines should be in the LEAK SUMMARY just before the "Rerun with..." line. It should be a single line: ==6281== Checked 1,123,768 bytes for pointers to 17 unfreed blocks. * "unfreed" rather than "not-freed"? * With all those changes: ==6310== HEAP SUMMARY: ==6310== in use at exit: 15,090 bytes in 17 blocks. ==6310== total heap usage: 17 allocs, 0 frees, 15,090 bytes allocated. ==6310== ==6310== LEAK SUMMARY: ==6310== definitely lost: 0 bytes in 0 blocks. ==6310== indirectly lost: 0 bytes in 0 blocks. ==6310== possibly lost: 0 bytes in 0 blocks. ==6310== still reachable: 10,694 bytes in 9 blocks. ==6310== suppressed: 4,396 bytes in 8 blocks. ==6310== Searched 1,123,768 bytes for pointers to 17 unfreed blocks. ==6310== Rerun with --leak-check=full to see details of leaked memory. ==6310== ==6310== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 3 from 3) ==6310== For counts of detected errors, rerun with: -v * I wonder if the HEAP SUMMARY is much use. The "in use at exit" bit overlaps partly with the leak summary and could be folded in there. And is the 1,123,768 bytes number useful? How about getting rid of the HEAP SUMMARY and just having this in the LEAK SUMMARY? ==6310== Searched for pointers to 17 unfreed blocks totalling 15,090 bytes. * About the --stats, I like the idea of untangling the mishmash that is -v. So why not take it further? So long as -v contains things that are useful to the user, any debugging info will be a distraction. We already have -d and it only contains debugging stuff. So how about we move all debugging info (eg. redirs, debug info) into -d, and only have useful user stuff (eg. suppressions used, full error list at the end) in -v? Also, -v -v stuff would be presumably moved into -d -d, and -v -v probably wouldn't need to exist as such. And then we could print -v stuff with "==pid==" rather than "--pid--", and do the same for error messages that the user needs to know about. N |