Port-i386 archive

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

Re: NET4501 w/options IPSEC breaks ipv6 at compile time?



David Laight <david%l8s.co.uk@localhost> writes:

> On Tue, Jun 19, 2012 at 01:27:58AM +0200, Joerg Sonnenberger wrote:
>> On Mon, Jun 18, 2012 at 07:03:05PM -0400, Greg Troxel wrote:
>> > 
>> > Any objections to me adding the NULL assignment?
>> 
>> That's exactly the reason why I don't like GCC's -Wuninitialize.
>> If code changes, changes are always that a real bug slipped through.
>
> The uninitialised variable warnings often do show up bugs, and
> blanket initialisations of variables to 0 or NULL can just stop
> the compiler finding bugs for you.

That's true (and what Joerg clarified to me in private mail), but two
questions arise:

  In this case, the code is woefully underdocumented, in that whether or
  not it is correct is non-obvious.  I can add a few comments,
  specifically the function that sets the frag pointer should claim that
  it doesn't read the pointer and always sets it.  But C doesn't have
  any way to express those rules in the language, so that will only be
  for humans.   Is there anything we can do to make such invariants
  manifest to the compiler?

  We have a case where the compile fails due to an uninitialized
  warning.  One option is to set the variable to NULL at declaration
  time, and perhaps to KASSERT that it is non-NULL at point of use.  I
  suspect other options are to turn down the warning level somehow, or
  tell the compiler not to complain about a particular variable.  But
  telling the compiler not to complain seems worse than NULL, because at
  least if there is a bug NULL will probably cause a segfault rather
  than arbitrary behavior.


So all in all, I'm in favor of setting variables to NULL to stop the
warning, only when the code change has been reviewed by others and
agreed not to be masking a bug.  To that end, first I'll add comments
that help the reviewer construct the argument of correctness.

Attachment: pgpysdNEk5xSM.pgp
Description: PGP signature



Home | Main Index | Thread Index | Old Index