> -----Original Message----- > From: Serge Hallyn [mailto:serge.hal...@ubuntu.com] > Sent: Wednesday, July 03, 2013 3:46 PM > To: Purcareata Bogdan-B43198 > Cc: lxc-devel@lists.sourceforge.net > Subject: Re: [lxc-devel] [PATCH] tests/startone: wrong set_cgroup_item check > > 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.
So lxcapi_set_cgroup_item() shouldn't return ret instead of (ret == 0)? That was my final point - I think the return value is "flipped" right at the end. > > > 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. Thank you, and sorry for all the noise :). I just wanted to make sure I understood how the return value propagates. > > -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
