Home page logo
/

nmap-dev logo Nmap Development mailing list archives

Re: Analysis of clang results against the nmap-npingchanges branch for nping directory only.
From: James Rogers <jamesmrogers () gmail com>
Date: Tue, 19 Jun 2012 11:20:25 -0400

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/

  By Date           By Thread  

Current thread:
[ Nmap | Sec Tools | Mailing Lists | Site News | About/Contact | Advertising | Privacy ]