[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v13 01/12] qcow2: Nicer variable names in qcow2_
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [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
signature.asc
Description: OpenPGP digital signature
- [Qemu-block] [PATCH v13 00/12] qcow2 zero-cluster tweaks [was add blkdebug tests], Eric Blake, 2017/05/06
- [Qemu-block] [PATCH v13 03/12] block: Update comments on BDRV_BLOCK_* meanings, Eric Blake, 2017/05/06
- [Qemu-block] [PATCH v13 04/12] qcow2: Correctly report status of preallocated zero clusters, Eric Blake, 2017/05/06
- [Qemu-block] [PATCH v13 05/12] qcow2: Name typedef for cluster type, Eric Blake, 2017/05/06
- [Qemu-block] [PATCH v13 06/12] qcow2: Make distinction between zero cluster types obvious, Eric Blake, 2017/05/06