Quoting Michael H. Warfield (m...@wittsend.com): > On Sun, 2011-06-26 at 14:00 -0500, Serge E. Hallyn wrote: > > Quoting Michael H. Warfield (m...@wittsend.com): > > > > Thanks, Michael, good catch. > > > > > > Now wait a minute. Is that a typo here: > > > > No it's not, but: > > > > > > char *s = index(retbuf, '.'); > > > > > > > > If you're doing, in effect, a dirname here should that be this: > > > > > > > > char *s = index(retbuf, '/'); > > > > > > > > IAC... That "*s = '\0';" should include a NULL check. > > > > > > > > Adding the NULL check and lxc-info works. > > > > > > > > Looks like that subsystem name in the call to that routine is not what > > > > Serge thought it was. I threw a print above the snprintf about just for > > > > giggles to print out the subsystem name being passed to it and this is > > > > what I got back... > > > > > > > > [mhw@forest SPECS]$ sudo lxc-info -n Alcove > > > > subsystem name: "freezer" > > > > 'Alcove' is RUNNING > > > > > > > > No wonder "s" was null. No dot and no /. > > > > > > I applied this patch and it got lxc-info working. But it was a quick > > > hack just to address the NULL pointer. Is it the correct fix? > > > > No, it's not. > > > > For the calls to this function that come from cgroup.c itself, '.' is the > > right thing. The problem is that lxc_cgroup_set() and lxc_cgroup_get() > > pass in things like 'devices.allow'. I was going to make the index > > conditional, but all the callers of this function pass in either a filename > > (with a '.' in it) or NULL. > > > > I failed to notice these: > > > > src/lxc/freezer.c: ret = lxc_cgroup_path_get(&nsgroup, "freezer", > > name); > > src/lxc/state.c: err = lxc_cgroup_path_get(&nsgroup, "freezer", > > name); > > > > :) > > > > These are what you are running into. > > > So the thing to do is leave it searching for index(s, '.') but do nothing > > if s is NULL. > > And that's what I believe results with my little hack. Only truncate if
Oops, sorry, I didn't look closely enough and assumed your patch was switching to checking for '/'. > there was a hit and s was non-null. I see now from your comments that > the check on '.' was correct. I was uncertain about the inputs and > outputs in this routine. Checking for the NILL condition may not be > "the" solution, in this case, but it is still a best common practice to > check pointers like that. Never can tell what may crop up in the > future. > > > Really it would be cleaner to have lxc_cgroup_{sg}et() do the index, so > > that lxc_cgorup_path_get() always gets a subsystem or NULL. I'm not doing > > that patch right now, though, trivial as it ought to be. > > I hear you. So Acked-by: Serge Hallyn <serge.hal...@ubuntu.com> to your patch to fix my bug, and let's leave it at that for now until it gets more testing? Thanks again for testing and looking into it! -serge ------------------------------------------------------------------------------ All of the data generated in your IT infrastructure is seriously valuable. Why? It contains a definitive record of application performance, security threats, fraudulent activity, and more. Splunk takes this data and makes sense of it. IT sense. And common sense. http://p.sf.net/sfu/splunk-d2d-c2 _______________________________________________ Lxc-users mailing list Lxc-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/lxc-users