Re: [Qemu-devel] [PATCH 0/5] block/dirty-bitmap: check number and size constraints against queued bitmaps

[John Snow]

On 6/6/19 5:54 PM, address@hidden wrote:
> Patchew URL: https://patchew.org/QEMU/address@hidden/
> 
> 
> 
> Hi,
> 
> This series seems to have some coding style problems. See output below for
> more information:
> 
> Subject: [Qemu-devel] [PATCH 0/5] block/dirty-bitmap: check number and size 
> constraints against queued bitmaps
> Type: series
> Message-id: address@hidden
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> git rev-parse base > /dev/null || exit 0
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> git config --local diff.algorithm histogram
> ./scripts/checkpatch.pl --mailback base..
> === TEST SCRIPT END ===
> 
> From https://github.com/patchew-project/qemu
>  * [new tag]               patchew/address@hidden -> patchew/address@hidden
> Switched to a new branch 'test'
> 6d00dc95dd block/qcow2-bitmap: Count queued bitmaps towards directory_size
> aaf1723431 block/qcow2-bitmap: Count queued bitmaps towards nb_bitmaps
> 475f5eef64 block/dirty-bitmap: rework bdrv_remove_persistent_dirty_bitmap
> 0698e46266 block/dirty-bitmap: Refactor bdrv_can_store_new_bitmap
> e94f44cb88 block/qcow2-bitmap: allow bitmap_list_load to return an error code
> 
> === OUTPUT BEGIN ===
> 1/5 Checking commit e94f44cb88bb (block/qcow2-bitmap: allow bitmap_list_load 
> to return an error code)
> ERROR: do not use assignment in if condition
> #151: FILE: block/qcow2-bitmap.c:1130:
> +    if ((ret = bitmap_list_load(bs, &bm_list, errp))) {
> 
> ERROR: do not use assignment in if condition
> #164: FILE: block/qcow2-bitmap.c:1189:
> +    if ((ret = bitmap_list_load(bs, &bm_list, errp))) {
> 
> total: 2 errors, 0 warnings, 166 lines checked
> 
> Patch 1/5 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 
> 2/5 Checking commit 0698e462661d (block/dirty-bitmap: Refactor 
> bdrv_can_store_new_bitmap)
> ERROR: do not use assignment in if condition
> #166: FILE: block/qcow2-bitmap.c:1656:
> +    if ((ret = bitmap_list_load(bs, &bm_list, errp))) {
> 
> total: 1 errors, 0 warnings, 200 lines checked
> 
> Patch 2/5 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 
> 3/5 Checking commit 475f5eef64a7 (block/dirty-bitmap: rework 
> bdrv_remove_persistent_dirty_bitmap)
> ERROR: do not use assignment in if condition
> #91: FILE: block/qcow2-bitmap.c:1421:
> +    if ((ret = bitmap_list_load(bs, &bm_list, errp))) {
> 
> total: 1 errors, 0 warnings, 143 lines checked
> 
> Patch 3/5 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 
> 4/5 Checking commit aaf172343148 (block/qcow2-bitmap: Count queued bitmaps 
> towards nb_bitmaps)
> ERROR: Missing Signed-off-by: line(s)
> 
> total: 1 errors, 0 warnings, 97 lines checked
> 
> Patch 4/5 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 
> 5/5 Checking commit 6d00dc95ddc0 (block/qcow2-bitmap: Count queued bitmaps 
> towards directory_size)
> === OUTPUT END ===
> 
> Test command exited with code: 1
> 
> 
> The full log is available at
> http://patchew.org/logs/address@hidden/testing.checkpatch/?type=message.
> ---
> Email generated automatically by Patchew [https://patchew.org/].
> Please send your feedback to address@hidden
> 

Whoops, script keeps dropping SoB for some reason. I also forgot that we
don't like assignment conditionals in QEMU, sorry :(

I'll fix these up, but I'll wait for other critique first so I don't
clog mailboxes.

--js