Hi Alan, Here are my comments. A cscope would have been helpful in making sure that the memory is being freed and that arguments match etc ... but I did the best I could.
usr/src/cmd/cmd-inet/lib/netcfgd/Makefile usr/src/cmd/cmd-inet/lib/netcfgd/netcfgd.c usr/src/cmd/cmd-inet/lib/nwamd/door_if.c I don't see why we have to use a constant and truncate the list. usr/src/cmd/cmd-inet/lib/nwamd/events.h usr/src/cmd/cmd-inet/lib/nwamd/ncu_phys.c usr/src/lib/libnwam/Makefile usr/src/lib/libnwam/common/libnwam.h usr/src/lib/libnwam/common/libnwam_audit.c usr/src/lib/libnwam/common/libnwam_backend.c line #229 -- How does strlen helps ?. it wont work if the string not null terminated in the first place usr/src/lib/libnwam/common/libnwam_enm.c* *usr/src/lib/libnwam/common/libnwam_events.c usr/src/lib/libnwam/common/libnwam_files.c It seems that after the call to nwam_line_to_object (line #478) the memory associated with proplist is not always freed. For example if the if statement at line 485 is entered but the name does not match nwam_free_object_list is not being called. line #815: Looking at line 794 Should objlist be freed ? usr/src/lib/libnwam/common/libnwam_impl.h Nit: Add an empty line b/w line #54 and #55 usr/src/lib/libnwam/common/libnwam_known_wlan.c line #159: get_wlans_cb() -- It does not immediately resets wil->list pointer after realloc(), so in case of error wil->list is not updated. line #200: nwam_walk_known_wlans() seems too complicated -- why does it have to call itself, why cant it simply call nwam_walk or get_wlans_cb directly. I am sure there is a good reason. Can some comments be added. usr/src/lib/libnwam/common/libnwam_loc.c usr/src/lib/libnwam/common/libnwam_ncp.c line# 180: nwam_ncp_create() should the handle be freed in case of an error or is the caller going to free it ? To many calls to other functions I need cscope to verify all memory is being freed. usr/src/lib/libnwam/common/libnwam_object.c line #144:nwam_walk() Why is objlist is not freed in all error cases. usr/src/lib/libnwam/common/libnwam_priv.h usr/src/lib/libnwam/common/libnwam_util.c usr/src/lib/libnwam/common/libnwam_values.c usr/src/lib/libnwam/common/libnwam_wlan.c usr/src/lib/libnwam/common/llib-lnwam usr/src/lib/libnwam/common/mapfile-vers I just saw your email about cscope. I am almost done now so hopefully these comments will help. Rao Alan Maguire wrote: > hi folks > > I'm requesting code review for > > http://defect.opensolaris.org/bz/show_bug.cgi?id=10245 > > Background - the doors interactions in libnwam were > broken. On the door server side, we were sometimes > alloca()ing a return argument, rather than having a client > pass in a maximally-sized buffer. In the course of fixing this > I've addressed another architectural issue relating > to the libnwam backend - namely that when we call > any of the walk() functions for nwam objects, the > walk was carried out by passing back an nvlist of nvlists, > where each nvlist was a property list for each object. > This obviously is problematic as the number of objects > grows. This approach has been amended to passing > back an nvlist containing a list of object names, each > of which is read() individually. This increases the door > traffic for walks, but with the performance improvements > that accrue from moving away from dynamic allocation, > the effects aren't apparent to the user. > > After all this I'm still seeing memory leaks in netcfgd unfortunately, > and these leaks are missing caller information, so ::bufctl_audit > doesn't shed any light on the origin. > > I'm also seeing a 24-byte memory leak in nwamcfg which > only happens when we "list" all objects (doesn't happen > for listing NCUs only for example). > > Any help in tracking down those leaks would be greatly > appreciated, and if any additional background is required > to make sense of these changes, don't hesitate to ask. > Thanks! > > Alan -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://mail.opensolaris.org/pipermail/nwam-dev/attachments/20090805/709d80d7/attachment.html>
