On Fri, 2007-08-24 at 08:13 -0400, James Carlson wrote:
> Garrett D'Amore writes:
> > > 28821 ipst = ns->netstack_ip;
> > > 28822 ASSERT(ipst != NULL);
> > > 28823
> > > 28824 ipst->ips_ip_cgtp_filter_ops = ops;
> [...]
> > 2) It provides better debug information when the programmer has made a
> > mistake. (The panic message and point of panic will be at the point of
> > assertion, rather than later in the code, which might be harder to
> > debug.)
>
> In this case, I'd argue that this is an unnecessary assertion.
>
> If the pointer were null, counter to the assertion, then we'd panic
> anyway at 28824, and the panic would be _obvious_. It'd be a "null
> pointer dereference" panic, and something that's trivial to diagnose
> with just a little effort.
>
> Thus, the ASSERT here adds no real value and shouldn't appear.
Ah, but the ASSERT here indicates that the author *knew* that he wasn't
checking for NULL, because it can't be. So it serves as a kind of
design statement. I think it is reasonable here. If it were removed,
then it should be replaced with a comment to the same effect...
-- Garrett
>
> Assertions *should* be used to validate design constraints that are
> hard to detect otherwise, such as a missing flag or state that will
> lead to trouble much further down the line, not something that'll fail
> in an obvious way on the very next line of code.
>
> If it helps, you can think of every pointer dereference as having an
> implicit pointer value check and assertion. This checks not just the
> single special value NULL, but also pointers that reference unmapped
> pages.
>
_______________________________________________
networking-discuss mailing list
[email protected]
