[asterisk-dev] Change in testsuite[master]: Memory Debugging Improvements

[Matt Jordan (Code Review)]

Matt Jordan has posted comments on this change.

Change subject: Memory Debugging Improvements
......................................................................


Patch Set 2:

(2 comments)

https://gerrit.asterisk.org/#/c/15/2/contrib/valgrind/text-summary.xsl
File contrib/valgrind/text-summary.xsl:

Line 4: <xsl:output method="text" omit-xml-declaration="yes" indent="no"/>
Looking at this XSLT, it is frustrating to have to use XML comments to properly format the stylesheet. That feels awkward.

This is clearly due to outputting this as text. While some of this could probably be eliminated by using concat(), places where you have conditional expressions are going to need 'help' to be formatted properly - so long as the output is text.

Why not output this as HTML? Having an HTML report of memory errors isn't a bad way to represent the output, and wouldn't necessitate the "comment" indentation. It also offers lots of advantages in terms of formatting the output, grouping it, linking it, etc.


https://gerrit.asterisk.org/#/c/15/2/lib/python/asterisk/asterisk.py
File lib/python/asterisk/asterisk.py:

Line 363:                 suppression_file = 'contrib/valgrind/suppressions.txt'
Asterisk also comes with a suppressions file. Is there a reason to not use that, or at least let the user specify the suppressions file to use?


-- 
To view, visit https://gerrit.asterisk.org/15
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I21634673508a01eea1f489c751d3cf75ea55cf06
Gerrit-PatchSet: 2
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-HasComments: Yes