Hi Vaughn,
I like the code checking too! When you decide adding all the bugs (grouped or not) to our bug tracker, I think you should add a specific tag "static code analysis" or something, such that they can easily be found.
Tav wrote:
Perhaps after submitting a few fixes, you can apply fixes directly to the code.
+1 (if Coverity allows you to do that?)
Tav wrote:
I do want to say that I really like your bug report. It clearly identifies the problem, the fix, and, most importantly for me, the rational for the fix.
Indeed, the bug report is quite clear (although it does not mention the (imho) real reason for the bug, C++ operator precedence, which may not be so clear to an inexperienced programmer). But I don't think you should write out such a report for each bug. (I'm guessing that the Coverity software provided the first part of the report?)
Great news!
Ciao, Johan
-----Original Message----- From: Tavmjong Bah [mailto:tavmjong@...8...] Sent: Tuesday, August 03, 2010 13:28
Hi Vaughn,
I think that it is a great offer to help identify and fix Inkscape bugs!! One that we should definitely facilitate. The question is how best to do this? The biggest problem, that I see, is that as Inkscape is developed by volunteers, the handling of hundreds of bugs may overwhelm the community! Here are some ideas that could help in this regard:
- Can the bugs be grouped by type and just one bug report be
opened for each type?
- Inkscape has a policy that after a person has had two
patches accepted the person is given write access to the code repository. Perhaps after submitting a few fixes, you can apply fixes directly to the code.
I do want to say that I really like your bug report. It clearly identifies the problem, the fix, and, most importantly for me, the rational for the fix.
TavOn Mon, 2010-08-02 at 23:10 -0700, Vaughn Spurlin wrote:
I've used Coverity's static source code analysis tools to identify hundreds of bugs in Inkscape source. I'd like some guidance on the best way to contribute bug report details and proposed fixes to Inkscape. I'm a relatively new Coverity employee getting up
to speed
with our static analysis tools for identifying defects.
Inkscape was
recommended as an open source project with C/C++ source
that I could
use to satisfy my educational needs while contributing to an open source project.
I want to contribute code fixes along with bug details. I suggest a first pass to report as many bugs as I can to the Inkscape Bug Tracker, followed by a second pass to add suggested fixes. Then Inkscape developers with commit privileges can review my code fix suggestions and apply the fixes as they see fit. Once the
first pass
is done and bug details are visible to the community,
anyone else is
welcome to contribute fixes also. What process should we follow to make the best use of everyone's efforts by avoiding several people working on the same bug?
What I'm requesting at this point is a discussion about my suggestions, and some decisions about how best to proceed. As an example, here's one defect found by Coverity analysis, with my proposed code fix.
CONSTANT_EXPRESSION_RESULT in /inkbugs/inkscape/src/main.cpp
In sp_do_export_png(SPDocument *...): An operation with
non-constant
operands that computes a result with constant value (CWE-569).
1345 if (sp_export_area) {
1346 /* Try to parse area (given in SVG pixels) */
1347 gdouble x0,y0,x1,y1;
!sscanf(sp_export_area, &"%lg:%lg:%lg:%lg", &x0, &y0, &x1,
&y1) == 4
is always false regardless of the values of its operands (logical operand of if).
1348 if (!sscanf(sp_export_area, "%lg:%lg:%lg:%lg",
&x0, &y0,
&x1, &y1) == 4) {
1349 g_warning("Cannot parse export area '%s'; use 'x0:y0:x1:y1'. Nothing exported.", sp_export_area);
1350 return;
1351 }
fix noted 2010-07-25:
1348 if (sscanf(sp_export_area, "%lg:%lg:%lg:%lg", &x0, &y0, &x1, &y1) != 4) {
fix reason:
sscanf returns the number of strings successfully scanned. The intent here is to ensure that all 4 input strings were scanned successfully.
Vaughn Spurlin
Coverity Technical Support