On Thu, 03 Dec 2009 11:55:26 +0000 Alan Maguire <Alan.Maguire at Sun.COM> wrote:
> I've got a few reviews to do for others today, but I thought > it'd be no harm to get this out there too: > > http://zhadum.east.sun.com/export/ws/amaguire/nwam1-rfes/webrev/ When you call die_nwamerr() you are careful to free things which have been allocated. But you don't closed fds or release other memory we've allocated. This seems inconsistent. We should have some policy here. Either we free nothing when we panic or we try to do a fairly through cleanup. I vote for the former as if the system is in an unknown state cleanup has little chance of making the reason any clearer. nwamcfg.c: In destroy_func you take into account the return value of object_name_from_handle() but if you get back a NULL realtime you deal with it. But in select_func() if you get a NULL back from object_name_from_handle it would fault. libnwam_files.c:373-378 MAXPATHLEN is a lot larger then MAXNAMELEN. It might be worth it to malloc(sizeof(NAME_CONFIG_DIR) + strlen(dp->d_name)+1) and then you can just memcpy(buf, NAME_CONFIG_DIR, sizeof (NAME_CONFIG_DIR)), strcpy(buf+sizeof(name_CONFIG_DIR), dp->d_name) since you've constructed the buffer to be the right size. You make up for the up front strcpy() by not having to use the heavy handed snprintf to concatenate strings. libnwam_files.c:508 How do you know filename is of length MAXPATHLEN? libnwam_ncp.c:174 This doesn't test either/or as the comment states. It tests that the name isn't automatic and that it is user. Michael > > Thanks! > > Alan > _______________________________________________ > nwam-dev mailing list > nwam-dev at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/nwam-dev
