Re: Analysis of clang results against the nmap-npingchanges branch for nping directory only.

[James Rogers <jamesmrogers () gmail com>]

Hi!

I added the no return attribute to several function definitions in
nmap-npingchanges/nping/output.h.  Then ran the clang analyzer against
nmap-npingchanges and got a report with many less bugs.

This totally eliminated the rest of the clang bug reports for
nmap-npingchanges/nping/.

I've attached a diff to this email, please review and see if you want
me to check these changes in.

I had never used an __attribute__ before, seems like something
interesting to learn about sometime soon.

Thanks!

James Rogers

On Wed, Jun 13, 2012 at 3:06 PM, James Rogers <jamesmrogers () gmail com> wrote:
> On Wed, Jun 13, 2012 at 2:09 PM, David Fifield <david () bamsoftware com> wrote:
> > On Wed, Jun 13, 2012 at 02:05:55PM -0400, James Rogers wrote:
> > > Only found two real issues:
> > >
> > > API           Argument with 'nonnull' attribute passed null   nping
> > > /common_modified.cc   490     20
> > > 490   s = strchr(addy[i],',');
> > >                  ^^^^
> > >       20 Null pointer passed as an argument to a 'nonnull' parameter
> > >
> > >       Is a bug.  hostexp is never checked to make sure it is not NULL.
> > >       371     char *hostexp = strdup(target_expr);
> > >       386     addy[0] = addy[1] = addy[2] = addy[3] = addy[4] = NULL;
> > >       387     addy[0] = r = hostexp;
> > >
> > >
> > > Dead store    Dead assignment                                 nping /EchoServer.cc            1507    1
> > > 1507  loopret=nsock_loop(nsp, 1000);
> > >
> > >       Value stored to 'loopret' is never read
> > >
> > >       This is a bug.  Easy to fix.
> > >       There were a lot more references to loopret that have been removed in
> > > this branch, this looks like the last one remaining.
> > >
> > > The rest of the bugs were generated by clang not recognizing that
> > > nping_fatal() and fatal() do not return.
> >
> > Is there a way to annotate these functions so that Clang can know they
> > don't return? In nmap_error.h we have for example,
> >
> > void fatal(const char *fmt, ...)
> >     __attribute__ ((noreturn))
> >
> > But that's missing in nping/output.h. If you add it, do those spurious
> > warnings go away?
> >
> > David Fifield
>
> The documentation talked about ways to mark things in general, but I
> didn't see any detailed explanation of how to do that.
>
> I'll add that attribute to the nmap-npingchanges version I have and
> see if it stops these bugs from being reported when I run the tool
> again.
>
> James Rogers

Attachment: AddedAttributeNoReturn.diff
Description:

_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://seclists.org/nmap-dev/