|
From: Nicholas N. <n.n...@gm...> - 2009-08-10 06:37:18
|
Hi, I'm looking at bug 197930 (https://bugs.kde.org/show_bug.cgi?id=197930) which seems simple, but fixing it requires fixing our rather ad hoc treatment of blank lines in the output. Current we put blank lines in various places: - at the end of the preamble - at the start of the postamble - before an error message, unless it's the first error message - before various other places It's ugly and messy, and the "unless" bit works fine so long as an error message is preceded by another error message, but otherwise we hit problems. In particular, leak errors are printed after the HEAP SUMMARY and whether we get a blank line or not depends on whether other errors were found earlier. So leak errors print an additional blank line to ensure that there's at least one after the HEAP SUMMARY, but this means that there are always two between each leak error. I propose a reworking that will fix 197930 and will make things a lot simpler. It involves establishing the following invariants: - print a blank at the end of the preamble (just like now) - print a blank at the start of the postamble (just like now) - for any significant user message, the code emitting the message should emit a blank line at its end. This makes the messaging code a lot simpler. One consequence can be seen in the output of memcheck/tests/manuel1.c (and a lot of others, this is just an example) which currently looks like this: ==23388== Memcheck, a memory error detector. ==23388== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al. ==23388== Using Valgrind-3.5.0.SVN and LibVEX; rerun with -h for copyright info ==23388== Command: memcheck/tests/manuel1 ==23388== ==23388== Conditional jump or move depends on uninitialised value(s) ==23388== at 0x40051B: main (manuel1.c:7) ==23388== ==23388== HEAP SUMMARY: ==23388== in use at exit: 0 bytes in 0 blocks. ==23388== total heap usage: 0 allocs, 0 frees, 0 bytes allocated. ==23388== ==23388== All heap blocks were freed -- no leaks are possible. ==23388== ==23388== For counts of detected and suppressed errors, rerun with: -v ==23388== Use --track-origins=yes to see where uninitialised values come from ==23388== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 8 from 1) With my proposed change there would be two blanks lines before the HEAP SUMMARY -- one after the error message, and one at the start of the postamble. That arguably makes it consistent with the no-error case, which currently looks like this (and would be the same with my proposal): ==23388== Command: memcheck/tests/manuel1 ==23388== ==23388== ==23388== HEAP SUMMARY: There are lots of regtest changes required with my proposal because with -q there's always a blank line printed after every error, which wasn't the case before. I don't think this is a big concern. Thoughts? Another possibility is to just get rid of the blank line at the start of the postamble, which requires more regtest .exp changes but is arguably even more consistent. The current approach, which attempts to avoid unnecessary blank lines, is admirable in its aim but is complex and doesn't work properly. It could be fixed up like this: - print a blank at the end of the preamble (just like now) - print a blank at the start of the postamble (just like now) - for any significant user message, the code emitting the message should emit a blank line at its start if any significant messages have been printed before. Things like the HEAP SUMMARY would count as significant messages. This is basically following the current design, but doing it properly. It would be a lot more complicated and fragile than my proposal above because you'd have to always record the fact that significant messages have been printed, and that would require touching a lot of places. In fact, I had a go at it but it's difficult to get working in all cases because of the need to always have the blank line at the start of the post-amble. N |