PVS Static Analysis

[wblock]

Summary

These are the static analysis results provided by Program Verification Systems. See https://lists.freebsd.org/pipermail/freebsd-doc/2016-February/026371.html

The review was run against r295235 (determined by des@, see https://lists.freebsd.org/pipermail/freebsd-doc/2016-February/026410.html).

[• hselasky]

I've fixed all the usb issues in sys/dev/usb . The others are false positives. Thank you!

[kp]

PVS-Studio-log-freebsd.txt

899

This looks like it might be a problem, added to my TODO list.

901

Doesn't look like a bug, we don't always allocate new rule, (but if we do it's never NULL because M_WAITOK), so it makes sense to NULL-check it.

902

This check is just wrong. Yes, we use *m before checking it, but that's because pf_test6() can change it.

903

Also seems to be wrong, we can only hit the NULL check from places where newpa is allocated.
We can probably get rid of the NULL check, especially considering that free() checks for NULL anyway.

904

size2 is never NULL, so we can get rid of the NULL check (perhaps adding an assert at the top of the function.

[rmacklem]

PVS-Studio-log-freebsd.txt

667

All of the "tl" ones are bogus. The NFSM_BUILD() and NFSM_DISSECT() macros
set tl to point to a block of memory the size of the argument. It is then incremented
through the block, sometimes skipping some bytes.

681

The case where it is used it is valid and non-null.

682

bp is only used when n > 0 and it is always valid and non-null when n > 0.

728

It is initialized before it is used.

[tuexen]

PVS-Studio-log-freebsd.txt

851

Addressed in https://svnweb.freebsd.org/changeset/base/295708.

856

Adressed in https://svnweb.freebsd.org/changeset/base/295708.

[tuexen]

PVS-Studio-log-freebsd.txt

862

This is not an issue, but the code has been cleaned up such that the warning will not show up anymore.
This is done in https://svnweb.freebsd.org/changeset/base/295709.

[tuexen]

PVS-Studio-log-freebsd.txt

846

False positive. If net != NULL then stcb =! NULL.

847

False positive.

848

False positive.

849

Addressed in https://svnweb.freebsd.org/changeset/base/295710.

850

Addressed in https://svnweb.freebsd.org/changeset/base/295710.

852

False positive.

853

False positive.

854

False positive.

855

False positive.

857

False positive.

858

False positive.

859

False positive.

860

False positive.

861

False positive.

866

False positive.

867

False positive.

869

False positive.

870

False positive.

871

False positive.

872

False positive.

873

False positive.

874

False positive.

875

False positive.

876

False positive.

[tuexen]

PVS-Studio-log-freebsd.txt

863

False positive.

864

False positive.

865

False positive.

868

False positive.

[markj]

PVS-Studio-log-freebsd.txt

37

The comment below tries to explain it: the check is bogus and is intended to prevent the compiler from making the dtrace_getarg() call a tail call. So this is a false positive.

I wonder if that trick is really reliable though. The check is only false if undefined behaviour has been invoked.

38

False positive. panic() doesn't return.

39

False positive; nspec > 0 => spec != NULL.

40

False positive; the code is assuming that enab->dten_ndesc > 0 implies that enab->dten_desc != NULL.

41

It does seem excessive, but I don't see why it might be wrong. It's also an assertion condition.

[marius]

PVS-Studio-log-freebsd.txt

269

False positive

558

False positive

[tuexen]

PVS-Studio-log-freebsd.txt

883

The bug reported can not happen, since the function is not really working as expected.
This is fixed in https://svnweb.freebsd.org/changeset/base/295771.

884

The bug reported can not happen, since the function is not really working as expected.
This is fixed in https://svnweb.freebsd.org/changeset/base/295771.

[imp]

PVS-Studio-log-freebsd.txt

29

False positive, but addressed in https://svnweb.freebsd.org/base?view=revision&revision=295759

[imp]

PVS-Studio-log-freebsd.txt

30

False positive. We always have a bp for a DELETE ccb, but the code is twisty enough that the static analyzer missed it.

32

False positive. serial_buf is always != NULL when have_serial is true, which we check in this loop.

[rmacklem]

PVS-Studio-log-freebsd.txt

732

It is initialized before used.

734

It is initialized before being used.

735

The if and else do the same thing, which is silly but not broken. I believe this
was inherited from the old NFS client, so I left the code that way, although I
did notice this.

736

This one I don't understand. "iv" is declared local to the function and is used
within the function, so I have no idea why it thinks there is a scope issue?

737

False positive, since the code checks for nfsd_master_proc != NULL
and then procp == nfsd_master_proc --> procp cannot be NULL.

768

It is initialized before being used.

787

It is initialized before being used.

[rmacklem]

667->788 are all false positives (no bugs identified), although #735 is silly and the
code should probably be cleaned up to take the if/else out someday.

PVS-Studio-log-freebsd.txt

788

Also a false positive. The only time nd == NULL is when the function is
called with CLOPS_RENEW. This code block isn't executed for CLOPS_RENEW.

[se]

PVS-Studio-log-freebsd.txt

359

Does no harm, but I'll fix it to simplify the loop condition where it occurs.

[se]

PVS-Studio-log-freebsd.txt

588

False positive: baddr != NULL is tested and only possible for vbp != NULL

589

Looks like a false positive: The value read into cp cannot be NULL unless there was a malloc error during driver attach (which had made the attach fail).

[op]

PVS-Studio-log-freebsd.txt

801

This is false positive, but moving "structsize = sizeof(elf_kinfo_proc_t);" before the check would fix this warning.
Not yet fixed / silenced this warning.

802

This is false positive, p used as dummy parameter at this call.

[trasz]

PVS-Studio-log-freebsd.txt

289

Hm, no idea what might be wrong here. False positive, perhaps? I'd appreciate if someone took a look at this.

[markj]

PVS-Studio-log-freebsd.txt

932

This one looks like a valid bug: ufs_dir_dd_ino() should set *dd_vp to NULL before returning in all cases.

[cy]

\sys\contrib\ipfilter\netinet\ip_rpcb_pxy.c (436): error V567: Undefined behavior. The 'p' variable is modified while being used twice between sequence points:

I would have coded this differently, using structs, making the code self documenting. This is OK as coded.

[cy]

\sys\contrib\ipfilter\netinet\ip_rpcb_pxy.c (912): error V567: Undefined behavior. The 'p' variable is modified while being used twice between sequence points:

This is OK.

[cy]

All these are OK.

\sys\contrib\ipfilter\netinet\ip_rpcb_pxy.c (994): error V567: Undefined behavior. The 'p' variable is modified while being used twice between sequence points.
\sys\contrib\ipfilter\netinet\ip_rpcb_pxy.c (436): error V567: Undefined behavior. The 'p' variable is modified while being used twice between sequence points.
\sys\contrib\ipfilter\netinet\ip_rpcb_pxy.c (912): error V567: Undefined behavior. The 'p' variable is modified while being used twice between sequence points.
\sys\contrib\ipfilter\netinet\ip_rpcb_pxy.c (877): error V567: Undefined behavior. The 'p' variable is modified while being used twice between sequence points.
\sys\contrib\ipfilter\netinet\ip_rpcb_pxy.c (549): error V567: Undefined behavior. The 'p' variable is modified while being used twice between sequence points.
\sys\contrib\ipfilter\netinet\ip_rpcb_pxy.c (438): error V567: Undefined behavior. The 'p' variable is modified while being used twice between sequence points.
\sys\contrib\ipfilter\netinet\ip_rpcb_pxy.c (437): error V567: Undefined behavior. The 'p' variable is modified while being used twice between sequence points.
\sys\contrib\ipfilter\netinet\ip_rpcb_pxy.c (895): error V567: Undefined behavior. The 'p' variable is modified while being used twice between sequence points.
\sys\contrib\ipfilter\netinet\ip_rpcb_pxy.c (891): error V567: Undefined behavior. The 'p' variable is modified while being used twice between sequence points.
\sys\contrib\ipfilter\netinet\ip_rpcb_pxy.c (433): error V567: Undefined behavior. The 'p' variable is modified while being used twice between sequence points.

[cy]

More false positives. The scanner failed to detect htons(), htonl(), ntohs() and ntohl().

\sys\contrib\ipfilter\netinet\ip_sync.c (358): error V523: The 'then' statement is equivalent to the 'else' statement.
\sys\contrib\ipfilter\netinet\ip_sync.c (330): error V523: The 'then' statement is equivalent to the 'else' statement.