On Thu, Jul 23, 2009 at 06:27:05PM +0100, Alan Maguire wrote: > thanks for the comments! responses inline...
Sounds good, thanks. -renee > Renee Danson Sommerfeld wrote: >> Hi Alan, >> >> Just a few comments. The first two are really code nits; the third >> is a point I'm not clear on. >> >> Thanks! >> renee >> >> nwamadm.c: rather than moving the check of the nwam service state out >> to the individual functions where it matters, could you make the >> check in main() conditional? That is, have a set of subcmds that >> require nwam to be online, and continue to do the state check in >> main() if the requested subcmd is in that set. >> >> > sure. I could add a boolean to the cmd structure > that specifies if the command requires nwamd > to be running. >> libnwam.h'57: not sure why this change was needed? >> >> > oops. I had thought I'd need another flag, > turned out I didn't. >> libnwam_loc.c'loc_set_enabled(): It looks like we just enable/disable >> as needed while walking the list of locations. > Yep. >> What happens if >> we hit the to-be-enabled location before other, currently enabled >> locations? Does this mean there will be a window where multiple >> locations are enabled? What's the consequence of that? >> >> > The first-encountered location is enabled, it won't > be an issue for the other location since as > part of the enable we do a request to nwamd to > enable the to-be-enabled location. Then the > to-be-disabled location will be marked as enabled=false. > So in other words, no negative consequences > as far as I can see. > > Thanks! > > Alan
