David Laight <david%l8s.co.uk@localhost> 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 parameter. > 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