Quoting Purcareata Bogdan-B43198 (b43...@freescale.com): > > -----Original Message----- > > From: Serge Hallyn [mailto:serge.hal...@ubuntu.com] > > Sent: Tuesday, July 02, 2013 5:27 PM > > To: Purcareata Bogdan-B43198 > > Cc: lxc-devel@lists.sourceforge.net > > Subject: Re: [lxc-devel] [PATCH] tests/startone: wrong set_cgroup_item check > > > > Quoting Bogdan Purcareata (bogdan.purcare...@freescale.com): > > > Minor typo. > > > > > > Signed-off-by: Bogdan Purcareata <bogdan.purcare...@freescale.com> > > > --- > > > src/tests/startone.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/src/tests/startone.c b/src/tests/startone.c > > > index d781e75..2c1f39b 100644 > > > --- a/src/tests/startone.c > > > +++ b/src/tests/startone.c > > > @@ -201,7 +201,7 @@ int main(int argc, char *argv[]) > > > > > > sprintf(buf, "FROZEN"); > > > b = c->set_cgroup_item(c, "freezer.state", buf); > > > - if (!b) { > > > + if (b) { > > > > Maybe I'm not thinking right, but why do you think setting freezer.state > > should fail? > > After looking at the lxcapi_set_cgroup_item implementation I'm a bit confused: > > src/lxc/lxccontainer.c: > 1383 static bool lxcapi_set_cgroup_item(struct lxc_container *c, const char > *subsys, const char *value) > 1384 { > 1385 int ret; > 1386 > [ ... ] initial checks > 1395 > 1396 ret = lxc_cgroup_set(c->name, subsys, value, c->config_path) == 0; > 1397 > 1398 container_disk_unlock(c); > 1399 return ret == 0; > 1400 } > > lxc_cgroup_set is basically a call to do_cgroup_set, which returns 0 (false) > on success (both functions are in src/lxc/cgroup.c). > > So calls return this: > > do_cgroup_set -> 0 (false) on success
don't say "0 (false)". It returns < 0 on error, 0 on success. > lxc_cgroup_set -> false on success returns 0 on success, < 0 on error. > ret (in lxcapi_set_cgroup_item) = (false == false) = true on success if lxc_cgroup_set() == 0, then lxc_cgroup_set() returned success. So return (lxc_cgroup_set() == 0) returns true on success. > lxcapi_set_cgroup_item -> (true == false) = false on success > > That's why I thought the check should have been made for (b) instead of (!b) > - since 1 (true) is the error case here. > Now I'm thinking maybe there should be a bit of refactoring in the code? But it's natural to anyone who's done kernel coding - and this is why it's clearly spelled out in the function preambles. The lxcapi (embodied in lxccontainer.c) is the bridge between various userspace codes and the more kernel-like lxc implementation, which is why it can be a bit tricky if you're not used to it. But that's why I went to the trouble of using bool rather than ints. If you have an int return value, hopefully 0 will be success. If bool, then true will be success. -serge ------------------------------------------------------------------------------ This SF.net email is sponsored by Windows: Build for Windows Store. http://p.sf.net/sfu/windows-dev2dev _______________________________________________ Lxc-devel mailing list Lxc-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/lxc-devel
