[libvirt] [PATCH 1/3] qemu: Add missing goto error in qemuRestoreCgroupState

Martin Kletzander mkletzan at redhat.com
Tue Dec 16 19:45:57 UTC 2014


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 at redhat.com>
>> ---
>>  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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20141216/9c06531f/attachment-0001.sig>


More information about the libvir-list mailing list