James Carlson wrote:
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.)
I will fix both. Thanks.
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.
I will do this one.
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?)
My x86 nightly had a linker error with ntohl() for kmdb, while sparc
build went through. Thats why I had to introduce the macros.
I got successful builds for both now.
Thanks
Vasumathi
_______________________________________________
networking-discuss mailing list
[email protected]
