Source-Changes-D archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: CVS commit: src/sys/netinet6

David Laight <> writes:

> On Mon, Mar 18, 2013 at 07:31:39PM +0000, Greg Troxel wrote:
>> Module Name: src
>> Committed By:        gdt
>> Date:                Mon Mar 18 19:31:39 UTC 2013
>> Modified Files:
>>      src/sys/netinet6: ip6_output.c
>> Log Message:
>> Initialize variable used as (conditional) result parameter.
>> ip6_insertfraghdr either sets a result parameter or returns an error.
>> While the caller only uses the result parameter in the non-error case,
>> knowing that requires cross-module static analysis, and that's not
>> robust against distant code changes.  Therfore, set ip6f to NULL
>> before the function call that maybe sets it, avoiding a spuruious
>> warning and changing the future possible bug from an unitialized
>> dereference to a NULL deferrence.
> 'If it returns fail it hasn't changed anything' is quite a common
> property of functions. In fact I'd tend to expect it.

Sure, but the property that's needed is the trickier one: being sure
that a function that doesn't return an error must have set a result

> Cross module analysis isn't really a big issue, the actual problem
> is when a compiler does a deeper analysis of a local function and
> fails to spot the relationship.

That may actually be the issue; I had a hard time convincing myself that
ip6_insertfraghdr follows it's own invariant.  The problem shows up if
one adds IPSEC to NET4501.

So if the compiler is sure, then the new NULL is a dead store and will
be optimized out.

Attachment: pgpY2gc9hg0VA.pgp
Description: PGP signature

Home | Main Index | Thread Index | Old Index