Re: [Qemu-block] [PATCH v13 01/12] qcow2: Nicer variable names in qcow2_update_snapshot_refcount()

[Eric Blake]

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 ;)

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature