On Tue, Dec 16, 2014 at 12:29:39PM -0700, Eric Blake wrote:
On 12/16/2014 12:13 PM, Martin Kletzander wrote:Commit af2a1f05 tried clearly separating each condition in qemuRestoreCgroupState() for the sake of readability, however somehow one condition body was missing. That means that the body of the next condition got executed only if both of there were true, which is impossible, thus resulting in a dead code and a logic error. Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx> --- src/qemu/qemu_cgroup.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 1e383c4..164ad05 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -801,6 +801,7 @@ qemuRestoreCgroupState(virDomainObjPtr vm) if ((empty = virCgroupHasEmptyTasks(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) < 0) + goto error; if (!empty) goto error;Why not the shorter: if ((empty - virCgroupHasEmptyTasks(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) <= 0) goto error; ACK to either style of fix.
As I said in the commit message, I wanted to clearly separate the conditions for the sake of readability. But you're right, because that bit me right back, I'll go with '<='. Thanks for the review, I pushed the series 9also with the fix in 3/3). Martin
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list