Vasumathi Sundaram writes:
> I fixed all the comments and the incremental webrev is at:
> http://cr.opensolaris.org/~vassun08/mdbmacro-webrev-r2/
[...]
> > usr/src/cmd/mdb/common/modules/ip/ip.c
[...]
> > 1680,1690: leaks allocated 'iw' structure.
> >
> 'iw' is stored in wsp->walk_data and freed in ipcl_hash_walk_fini()
No, it's not. It's only stored in wsp->walk_data at line 1697 (in the
new webrev above). At lines 1681 and 1691, the function returns
*without* storing the 'iw' pointer anywhere and without freeing the
storage.
It's a leak, though a small one.
> > 1711: careful: check the size of the structure. If it's "big," then
> > allocate with mdb_alloc instead of using on the stack. (This is a
> > general mdb/kmdb issue; make sure you inspect all of your functions
> > for excessive stack usage, because they can panic the system if used
> > in kmdb, as I learned recently and rather painfully.)
> >
> The size of conn_t is large (>1K). I changed it use mdb_alloc.
There's a new (and much more serious) memory leak at lines 1717, 1728,
1730, and 1733: 'conn' isn't ever freed.
(Consider putting 'conn' inside ipcl_hash_walk_data_t rather than
allocating it and freeing it anew on each step.)
> > usr/src/uts/common/inet/ip/ip.c
> >
> > 775,28358-28385: more should be removed to get code orphaned by this
> > change.
> >
> > 917-918: shouldn't this go as well?
> >
> The data structure srcid_map_t (ip_stack_t:ip_srcid_head) is opaque,
> defined and used only within ip_srcid.c
> I could not pull this to mdb.
I'd very much prefer that we don't leave any of this ndd status report
swill behind for a future clean-up effort. There are at least two
very simple fixes for this:
1. Move struct srcid_map and srcid_map_t out of ip_srcid.c and put
them in inet/ip.h with the rest of the general IP kernel data
structures. That will give mdb access to the structure.
or:
2. Just ditch the srcid map printing functions. It's possible that
this has just never been used.
I don't really have a strong preference between those to.
> I fixed another problem in function mask_to_prefixlen(). I introduced a
> macro to get host-byte-ordered mask values.
What is this other problem that you've fixed? What was wrong with the
ntohl() call that was there before? (kmdb issue?)
--
James Carlson, Solaris Networking <[email protected]>
Sun Microsystems / 35 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]
