On Fri, 2007-08-24 at 10:50 +0800, Jason Jiang - Solaris China Team
wrote:
> Hi Eric,
>
> I read the code as a learning experience. :-)
> I have a question about your code about ASSERT:
>
> ip.c:
> 28821 ipst = ns->netstack_ip;
> 28822 ASSERT(ipst != NULL);
> 28823
> 28824 ipst->ips_ip_cgtp_filter_ops = ops;
>
> We know the ASSERT will take effect on debug version only. And it do nothing
> on release version(Am I right?).
> The question is, it seems that ipst is possible to be a NULL for you are
> using the ASSERT to check it.
> How about on release version? Can we say it will panic here?
The point of ASSERT is two-fold.
1) It documents explicitly the conditions that a programmer is assuming
*must* hold true.
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.)
Anyway, if ipst can *ever* reasonably (or even unreasonably) be NULL
here, then it would be a gross programmer to use ASSERT here. ASSERT
causes a PANIC when it fails. (I'm assuming Erik knows what he is doing
here, and really does want to ASSERT that ipst at this *particular*
place in the code cannot ever be NULL.)
In this particular case, if ASSERT is not compiled in, the code will
*still* panic if ipst is NULL, it just will do so at 28824.
Understand that having the ASSERT won't avoid a panic on a debug kernel,
it will just make the panic easier to debug.
-- Garrett
>
> Thanks,
> Jason
>
>
> Erik Nordmark åé:
> > The CGTP (Carrier Grade Transport Protocol) support in IP needs to be
> > extended to allow separate configuration of CGTP for different
> > exclusive-IP zones.
> >
> > This is:
> > PSARC 2007/250 CGTP for IP Instances
> > 6572777 Need CGTP hooks version 3 to allow per-IP instance hooks
> >
> > The webrev is at:
> > http://cr.grommit.com/~nordmark/cgtpv3/
> >
> > Who can code review this for me?
> >
> > Thanks,
> > Erik
> >
> >
> > _______________________________________________
> > networking-discuss mailing list
> > [email protected]
> >
>
> _______________________________________________
> networking-discuss mailing list
> [email protected]
_______________________________________________
networking-discuss mailing list
[email protected]
