qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v13 01/12] qcow2: Nicer variable names in qcow2_


From: Max Reitz
Subject: Re: [Qemu-devel] [PATCH v13 01/12] qcow2: Nicer variable names in qcow2_update_snapshot_refcount()
Date: Mon, 8 May 2017 17:37:57 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.0.1

On 08.05.2017 17:36, Eric Blake wrote:
> On 05/08/2017 10:27 AM, Max Reitz wrote:
>> On 07.05.2017 02:05, Eric Blake wrote:
>>> In order to keep checkpatch happy when the next patch changes
>>> indentation, we first have to shorten some long lines.  The easiest
>>> approach is to use a new variable in place of
>>> 'offset & L2E_OFFSET_MASK', except that 'offset' is the best name
>>> for that variable.  Change '[old_]offset' to '[old_]entry' to
>>> make room.
>>>
>>> While touching things, also fix checkpatch warnings about unusual
>>> 'for' statements.
>>>
>>> Suggested by Max Reitz <address@hidden>
>>> Signed-off-by: Eric Blake <address@hidden>
>>>
> 
>>> @@ -1117,20 +1118,22 @@ int qcow2_update_snapshot_refcount(BlockDriverState 
>>> *bs,
>>>                  goto fail;
>>>              }
>>>
>>> -            for(j = 0; j < s->l2_size; j++) {
>>> +            for (j = 0; j < s->l2_size; j++) {
>>>                  uint64_t cluster_index;
>>> +                uint64_t offset;
>>
>> Actually, introducing entry and old_entry in this scope would have
>> sufficed, too, because they aren't used anywhere else. But nobody
>> actually cares (O:-)), so:
>>
>> Reviewed-by: Max Reitz <address@hidden>
>>
>>>
>>> -                offset = be64_to_cpu(l2_table[j]);
>>> -                old_offset = offset;
>>> -                offset &= ~QCOW_OFLAG_COPIED;
>>> +                entry = be64_to_cpu(l2_table[j]);
>>> +                old_entry = entry;
> 
> Other things I thought about, but didn't care enough to actually do:
> it's possible to reduce a line of code by either:
> 
> old_entry = entry = be64_to_cpu(l2_table[j]);
> entry &= ~QCOW_OFLAG_COPIED;
> 
> or by:
> 
> old_entry = be64_to_cpu(l2_table[j]);
> entry = old_entry & ~QCOW_OFLAG_COPIED;
> 
> for one less line of source.  You're welcome to make either such tweak
> if you care, but I'm not going to respin for just that ;)

No, I actually like the code as it is. :-)

Max

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]