Peter Memishian writes: > In general, looks good. A few comments on common/inet/ip/ip_if.c: > > * The *_untrace_ref() functions seem to assume that the thread > doing the untrace will already be known to the tracing > framework (or we'll panic). I don't see what guarantees this.
They do indeed assume this; it's guaranteed by construction and usage of *_REFHOLD and *_REFRELE. (Answered on a separate thread.) > * The *_trace_ref() functions ASSERT() that mod_hash_insert() > cannot fail, but I don't see what makes that a safe assertion. > Seems like this will panic under sufficient memory pressure. Yes. I'll fix that. > * Speaking of the *_trace_ref() functions: seems like a lot of > the code could be shared -- e.g., we could have a common > th_trace_ref() function such as: Accepted; will make common functions. > * I'm not sure why this: > > #ifdef DEBUG > ipif_trace_cleanup(ipif); > #endif > ... is seen as preferable to: > > IPIF_TRACE_CLEANUP(ipif) > > The latter seems less obtrusive. I can switch it back if you prefer. The reason I made that change is that there's really only one place where it's invoked outside of a big ifdef DEBUG block, so having a special wrapper macro (and one with "global" visibility at that) for it seemed silly. It just obscures the function calls. -- James Carlson, Solaris Networking <[EMAIL PROTECTED]> Sun Microsystems / 1 Network Drive 71.232W Vox +1 781 442 2084 MS UBUR02-212 / Burlington MA 01803-2757 42.496N Fax +1 781 442 1677 _______________________________________________ networking-discuss mailing list [email protected]
