[prev in list] [next in list] [prev in thread] [next in thread]
List: sonar-dev
Subject: Re: [sonar-dev] False Positives Widget, C Plugin
From: Dinesh Bolkensteyn <dinesh.bolkensteyn () sonarsource ! com>
Date: 2012-05-30 10:27:57
Message-ID: CAA06TwPcgZwsC5C0SNBqvPE6j3oDQbQqZW-pPqTkdDKUWsQNqw () mail ! gmail ! com
[Download RAW message or body]
Hi Luca,
Thank you for your feedback!
See my answers inline hereunder.
--
Dinesh Bolkensteyn
www.SonarSource.com <http://www.sonarsource.com/>
twitter.com/DBolkensteyn
On Wed, May 30, 2012 at 7:32 AM, Luca Longinotti <chtekk@gmail.com> wrote:
> Hi, first of all a big thank you for developing Sonar, it's been quite
> helpful in several projects and works very well.
> Still, I do have a few issues that I've noticed to report:
>
> Sonar
>
> 1) The False Positives widget in the Reviews Dashboard doesn't seem to
> work correctly when there are more false positives that what it can
> display right away: links to successive pages which should contain the
> other false positives are created, but clicking on them doesn't
> actually get you anywhere. For an example, go to
> https://dev.longitekk.com/sonar/dashboard/index/127?did=3
> and click on "2" or "Next" in the False Positives widget; the links
> disappear but you don't get the next 5 false positives displayed!
>
Indeed this is a bug in Sonar itself, which I was able to reproduce in
Sonar 3.0, ticket created https://jira.codehaus.org/browse/SONAR-3528
>
> Sonar C Plugin
>
> 1) The "C.FunctionName" rule doesn't seem to resolve macros, leading to
> false positives on definitions like START_TEST(my_func_name) { ... }
> See
>
> https://dev.longitekk.com/sonar/drilldown/violations/12?&rule=c:C.FunctionName&rule_sev=MAJOR
> for an example.
>
All rules beside preprocessor-level ones are executed against the
preprocessor code.
Therefore, the "C.FunctionName" rule is not supposed to see
"START_TEST(...)" if START_TEST is defined as a macro.
It seems that "START_TEST" is not defined in the included file "tests.h" (
https://dev.longitekk.com/Rig/browser/trunk/tests/tests.h)
Therefore I assume that the defintion is either in <rig.h> or <check.h>
For the include to work with the latest released version of the C plugin,
you must have either:
1) Configured the analysis with a source folder with (recursively)
contain a file "rig.h" or "check.h"
2) Added the folder which contains the file "rig.h" or "check.h" as an
additional library folder, which serves the same purpose for our plugin as
the -I option of GCC.
This include resolving strategy is not optimal we are working on improving
it, see for example: http://jira.sonarsource.com/browse/CANSI-102
>
> 2) The "C.EmptyBlock" rule doesn't seem to get #error in #ifdef
> blocks correctly, like #if ... #elif ... #else #error ... #endif, there
> is no empty block here technically, since either the ifs are taken,
> which contain code, or you error out and don't even get past
> pre-processing stage. Maybe the C plugin doesn't treat the #error
> pre-processor directive correctly?
>
> https://dev.longitekk.com/sonar/drilldown/violations/12?&rule=c:C.EmptyBlock&rule_sev=MAJOR
> on rig_misc.c shows this.
>
The preprocessor seems to have reached the #error directive, which
currently generates only a warning (it does not lead to an analysis
failure).
Therefore, the rule indeed sees an empty block.
I see two options for solving this issue:
1) Fix the includes as above and hopefully the SYSTEM_* macros will be
seen as defined
2) Use the predefined macro property to force the definition of one of
the given SYSTEM_* macro.
>
> 3) The "C.NamedParameters" rule doesn't interpret function pointer
> parameters correctly, it always says they have no name.
>
> https://dev.longitekk.com/sonar/drilldown/violations/12?&rule=c:C.NamedParameters&rule_sev=MAJOR
> for examples, it only happens on functions which have function pointers
> in their signature.
>
This rule is intended to be implemented as specified by MISRA.
According to http://www.misra.org.uk/forum/viewtopic.php?f=76&t=1128
It seems that function pointer parameters must be named.
Therefore, they recommend that you refactor: void foo(int (*cmp)(int,
int)); // Alternative 1: Function prototype without function pointer
arguments names
to: void foo(int (*cmp)(int a, int b)); // Function prototype with function
pointer arguments names
>
> 4) I'm getting parser failure on a few files,
>
> https://dev.longitekk.com/sonar/drilldown/violations/12?&rule=c:C.ParsingError&rule_sev=MAJOR
> for the list, as far as I can tell the C Plugin doesn't like __asm__
> __volatile__ snippets, which are the recommended way to write down
> inline assembler in GCC. The atomic_ops_gcc_* files aren't even lexed
> correctly, not sure if it's only this problem with __asm__ __volatile__
> or also related to (2), because I use #error in global scope there.
>
1) atomic_ops_gcc_sparcv9.c, due to
http://jira.sonarsource.com/browse/CANSI-132
2) atomic_ops_gcc_x86-64.c, also due to
http://jira.sonarsource.com/browse/CANSI-132
3) cpuid.c, ticket created, http://jira.sonarsource.com/browse/CANSI-148
>
> 5) The rule "C.InnerC99Comments" triggers on any kind of URL in a
> comment too, is this intended? See
>
> https://dev.longitekk.com/sonar/drilldown/violations/12?&rule=c:C.InnerC99Comments&rule_sev=MINOR
> for examples.
>
This is indeed intended. If it does not work for you, then you probably
want to disable this rule.
>
> 6) Rule "C.ForLoopCounter" seems to generate false positives on
> dereference. I have loops like
> for (size_t i = 0; i < struct->field; i++) { ... }
> and it marks them as a violation, but this seems completely correct to
> me. See
>
> https://dev.longitekk.com/sonar/drilldown/violations/12?&rule=c:C.ForLoopCounter&rule_sev=MINOR
> for examples.
>
It also seems correct to me, ticket created,
http://jira.sonarsource.com/browse/CANSI-149
>
> Ok that's it, everything I've found up till now. If you want me to file
> bugs for those issues, once confirmed/discussed, just tell me.
> Have a nice day everyone!
>
Thank you, have a nice day too.
> --
> Luca Longinotti (chtekk)
>
> chtekk@longitekk.com
> luca@longinotti.ch
>
> ---------------------------------------------------------------------
> To unsubscribe from this list, please visit:
>
> http://xircles.codehaus.org/manage_email
>
>
>
[Attachment #3 (text/html)]
<div>Hi Luca,</div><div> </div><div>Thank you for your feedback!</div><div> \
</div><div>See my answers inline hereunder.<br clear="all"><span \
style="font-family:arial,sans-serif;font-size:13px;border-collapse:collapse"><br>
--<br>Dinesh Bolkensteyn<br></span><span \
style="font-family:arial,sans-serif;font-size:13px;border-collapse:collapse"><a \
href="http://www.sonarsource.com/" \
target="_blank">www.SonarSource.com</a></span><br><a \
href="http://twitter.com/DBolkensteyn" \
target="_blank">twitter.com/DBolkensteyn</a><br>
<br></div><div class="gmail_quote">On Wed, May 30, 2012 at 7:32 AM, Luca Longinotti \
<span dir="ltr"><<a href="mailto:chtekk@gmail.com" \
target="_blank">chtekk@gmail.com</a>></span> wrote:<br><blockquote \
style="margin:0px 0px 0px \
0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid" \
class="gmail_quote">
Hi, first of all a big thank you for developing Sonar, it's been quite<br>
helpful in several projects and works very well.<br>
Still, I do have a few issues that I've noticed to report:<br>
<br>
Sonar<br>
<br>
1) The False Positives widget in the Reviews Dashboard doesn't seem to<br>
work correctly when there are more false positives that what it can<br>
display right away: links to successive pages which should contain the<br>
other false positives are created, but clicking on them doesn't<br>
actually get you anywhere. For an example, go to<br>
<a href="https://dev.longitekk.com/sonar/dashboard/index/127?did=3" \
target="_blank">https://dev.longitekk.com/sonar/dashboard/index/127?did=3</a><br> and \
click on "2" or "Next" in the False Positives widget; the \
links<br> disappear but you don't get the next 5 false positives \
displayed!<br></blockquote><div> </div><div><font color="#ff0000">Indeed this is a \
bug in Sonar itself, which I was able to reproduce in Sonar 3.0, ticket created <a \
href="https://jira.codehaus.org/browse/SONAR-3528" \
target="_blank">https://jira.codehaus.org/browse/SONAR-3528</a></font></div>
<div> </div><blockquote style="margin:0px 0px 0px \
0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid" \
class="gmail_quote"> <br>
Sonar C Plugin<br>
<br>
1) The "C.FunctionName" rule doesn't seem to resolve macros, leading \
to<br> false positives on definitions like START_TEST(my_func_name) { ... }<br>
See<br>
<a href="https://dev.longitekk.com/sonar/drilldown/violations/12?&rule=c:C.FunctionName&rule_sev=MAJOR" \
target="_blank">https://dev.longitekk.com/sonar/drilldown/violations/12?&rule=c:C.FunctionName&rule_sev=MAJOR</a><br>
for an example.<br></blockquote><div> </div><div><font color="#ff0000">All rules \
beside preprocessor-level ones are executed against the preprocessor \
code.</font></div><div><font color="#ff0000"></font> </div><div><font \
color="#ff0000">Therefore, the "C.FunctionName" rule is not supposed to see \
"START_TEST(...)" if START_TEST is defined as a macro.</font></div>
<div> </div><div><font color="#ff0000">It seems that "START_TEST" is not \
defined in the included file "tests.h" (<a \
href="https://dev.longitekk.com/Rig/browser/trunk/tests/tests.h" \
target="_blank">https://dev.longitekk.com/Rig/browser/trunk/tests/tests.h</a>)</font></div>
<div><font color="#ff0000"></font> </div><div><font color="#ff0000">Therefore I \
assume that the defintion is either in <rig.h> or \
<check.h></font></div><div><font color="#ff0000"></font> </div><div><font \
color="#ff0000">For the include to work with the latest released version of the C \
plugin, you must have either:</font></div>
<div><font color="#ff0000"></font> </div><div><font color="#ff0000"> 1) Configured \
the analysis with a source folder with (recursively) contain a file "rig.h" \
or "check.h"</font></div><div><font color="#ff0000"> 2) Added the folder \
which contains the file "rig.h" or "check.h" as an additional \
library folder, which serves the same purpose for our plugin as the -I option of \
GCC.</font></div>
<div><font color="#ff0000"></font> </div><div><font color="#ff0000">This include \
resolving strategy is not optimal we are working on improving it, see for example: <a \
href="http://jira.sonarsource.com/browse/CANSI-102" \
target="_blank">http://jira.sonarsource.com/browse/CANSI-102</a></font></div>
<div> </div><blockquote style="margin:0px 0px 0px \
0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid" \
class="gmail_quote"> <br>
2) The "C.EmptyBlock" rule doesn't seem to get #error in #ifdef<br>
blocks correctly, like #if ... #elif ... #else #error ... #endif, there<br>
is no empty block here technically, since either the ifs are taken,<br>
which contain code, or you error out and don't even get past<br>
pre-processing stage. Maybe the C plugin doesn't treat the #error<br>
pre-processor directive correctly?<br>
<a href="https://dev.longitekk.com/sonar/drilldown/violations/12?&rule=c:C.EmptyBlock&rule_sev=MAJOR" \
target="_blank">https://dev.longitekk.com/sonar/drilldown/violations/12?&rule=c:C.EmptyBlock&rule_sev=MAJOR</a><br>
on rig_misc.c shows this.<br></blockquote><div> </div><div><font color="#ff0000">The \
preprocessor seems to have reached the #error directive, which currently generates \
only a warning (it does not lead to an analysis failure).</font></div>
<div><font color="#ff0000"></font> </div><div><font color="#ff0000">Therefore, the \
rule indeed sees an empty block.</font></div><div><font color="#ff0000"></font> \
</div><div><font color="#ff0000">I see two options for solving this \
issue:</font></div>
<div><font color="#ff0000"></font> </div><div><font color="#ff0000"> 1) Fix the \
includes as above and hopefully the SYSTEM_* macros will be seen as \
defined</font></div><div><font color="#ff0000"></font> </div><div><font \
color="#ff0000"> 2) Use the predefined macro property to force the definition of one \
of the given SYSTEM_* macro.</font></div>
<div> </div><blockquote style="margin:0px 0px 0px \
0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid" \
class="gmail_quote"> <br>
3) The "C.NamedParameters" rule doesn't interpret function pointer<br>
parameters correctly, it always says they have no name.<br>
<a href="https://dev.longitekk.com/sonar/drilldown/violations/12?&rule=c:C.NamedParameters&rule_sev=MAJOR" \
target="_blank">https://dev.longitekk.com/sonar/drilldown/violations/12?&rule=c:C.NamedParameters&rule_sev=MAJOR</a><br>
for examples, it only happens on functions which have function pointers<br>
in their signature.<br></blockquote><div> </div><div><font color="#ff0000">This rule \
is intended to be implemented as specified by MISRA.</font></div><div><font \
color="#ff0000"></font> </div><div><font color="#ff0000">According to <a \
href="http://www.misra.org.uk/forum/viewtopic.php?f=76&t=1128" \
target="_blank">http://www.misra.org.uk/forum/viewtopic.php?f=76&t=1128</a></font></div>
<div><font color="#ff0000"></font> </div><div><font color="#ff0000">It seems that \
function pointer parameters must be named.</font></div><div><font \
color="#ff0000"></font> </div><div><font color="#ff0000">Therefore, they recommend \
that you refactor: </font><font color="#000000">void foo(int (*cmp)(int, int)); // \
Alternative 1: Function prototype without function pointer arguments \
names</font></div>
<div><font color="#ff0000">to: </font><font color="#000000">void foo(int (*cmp)(int \
a, int b)); // Function prototype with function pointer arguments \
names</font></div><div> </div><blockquote style="margin:0px 0px 0px \
0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid" \
class="gmail_quote">
<br>
4) I'm getting parser failure on a few files,<br>
<a href="https://dev.longitekk.com/sonar/drilldown/violations/12?&rule=c:C.ParsingError&rule_sev=MAJOR" \
target="_blank">https://dev.longitekk.com/sonar/drilldown/violations/12?&rule=c:C.ParsingError&rule_sev=MAJOR</a><br>
for the list, as far as I can tell the C Plugin doesn't like __asm__<br>
__volatile__ snippets, which are the recommended way to write down<br>
inline assembler in GCC. The atomic_ops_gcc_* files aren't even lexed<br>
correctly, not sure if it's only this problem with __asm__ __volatile__<br>
or also related to (2), because I use #error in global scope \
there.<br></blockquote><div> </div><div><font color="#ff0000">1) \
atomic_ops_gcc_sparcv9.c, due to </font><a \
href="http://jira.sonarsource.com/browse/CANSI-132" target="_blank"><font \
color="#ff0000">http://jira.sonarsource.com/browse/CANSI-132</font></a></div>
<div><font color="#ff0000">2) atomic_ops_gcc_x86-64.c, also due to </font><a \
href="http://jira.sonarsource.com/browse/CANSI-132" target="_blank"><font \
color="#ff0000">http://jira.sonarsource.com/browse/CANSI-132</font></a><font \
color="#ff0000"> </font></div>
<div><font color="#ff0000">3) cpuid.c, ticket created, </font><a \
href="http://jira.sonarsource.com/browse/CANSI-148" target="_blank"><font \
color="#ff0000">http://jira.sonarsource.com/browse/CANSI-148</font></a></div><div>
</div><blockquote style="margin:0px 0px 0px \
0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid" \
class="gmail_quote">
<br>
5) The rule "C.InnerC99Comments" triggers on any kind of URL in a<br>
comment too, is this intended? See<br>
<a href="https://dev.longitekk.com/sonar/drilldown/violations/12?&rule=c:C.InnerC99Comments&rule_sev=MINOR" \
target="_blank">https://dev.longitekk.com/sonar/drilldown/violations/12?&rule=c:C.InnerC99Comments&rule_sev=MINOR</a><br>
for examples.<br></blockquote><div> </div><div><font color="#ff0000"></font> <font \
color="#ff0000">This is indeed intended. If it does not work for you, then you \
probably want to disable this rule.</font></div><div> </div>
<blockquote style="margin:0px 0px 0px \
0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid" \
class="gmail_quote"> <br>
6) Rule "C.ForLoopCounter" seems to generate false positives on<br>
dereference. I have loops like<br>
for (size_t i = 0; i < struct->field; i++) { ... }<br>
and it marks them as a violation, but this seems completely correct to<br>
me. See<br>
<a href="https://dev.longitekk.com/sonar/drilldown/violations/12?&rule=c:C.ForLoopCounter&rule_sev=MINOR" \
target="_blank">https://dev.longitekk.com/sonar/drilldown/violations/12?&rule=c:C.ForLoopCounter&rule_sev=MINOR</a><br>
for examples.<br></blockquote><div> </div><div><font color="#ff0000">It also seems \
correct to me, ticket created, <a \
href="http://jira.sonarsource.com/browse/CANSI-149">http://jira.sonarsource.com/browse/CANSI-149</a></font></div>
<div> </div><blockquote style="margin:0px 0px 0px \
0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid" \
class="gmail_quote"> <br>
Ok that's it, everything I've found up till now. If you want me to file<br>
bugs for those issues, once confirmed/discussed, just tell me.<br>
Have a nice day everyone!<br></blockquote><div> </div><div><font \
color="#ff0000">Thank you, have a nice day too.</font></div><div> </div><blockquote \
style="margin:0px 0px 0px \
0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid" \
class="gmail_quote">
<span><font color="#888888">--<br>
Luca Longinotti (chtekk)<br>
<br>
<a href="mailto:chtekk@longitekk.com" target="_blank">chtekk@longitekk.com</a><br>
<a href="mailto:luca@longinotti.ch" target="_blank">luca@longinotti.ch</a><br>
<br>
---------------------------------------------------------------------<br>
To unsubscribe from this list, please visit:<br>
<br>
<a href="http://xircles.codehaus.org/manage_email" \
target="_blank">http://xircles.codehaus.org/manage_email</a><br> <br>
<br>
</font></span></blockquote></div><br>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic