Re: [PATCH v2 13/44] qemu-option: Use returned bool to check for failure
02.07.2020 18:49, Markus Armbruster wrote:
The previous commit enables conversion of
foo(..., &err);
if (err) {
...
}
to
if (!foo(..., &err)) {
...
}
for QemuOpts functions that now return true / false on success /
error. Coccinelle script:
@@
identifier fun = {opts_do_parse, parse_option_bool, parse_option_number,
parse_option_size, qemu_opt_parse, qemu_opt_rename, qemu_opt_set,
qemu_opt_set_bool, qemu_opt_set_number, qemu_opts_absorb_qdict,
qemu_opts_do_parse, qemu_opts_from_qdict_entry, qemu_opts_set,
qemu_opts_validate};
expression list args, args2;
typedef Error;
Error *err;
@@
-fun(args, &err, args2);
-if (err)
+if (!fun(args, &err, args2))
{
...
}
A few line breaks tidied up manually.
Signed-off-by: Markus Armbruster
Improve reviewer script to cover indentation/spacing/wrapping changes:
#!/usr/bin/env python3
import sys
import re
re_remove_add = re.compile(r'(?P(^-.*\n)+)(?P(^\+.*\n)+)',
flags=re.MULTILINE)
re_remove = re.compile(r'(?P(?P\w+)\(.*,&(?P\w+)\));'
r'if\((?P=err)(!=NULL)?\)\{')
re_add = re.compile(r'if\(!(?P.*)\)\{')
all_functions = set()
def check_subchunk(subchunk):
"""check subchunk
Check that subchunk matches
- func(args..., &err);
- if (err) {
+ if (!func(args..., &err)) {
ignoring indentation and spaces
Also, while being here, register each found func in all_functions.
"""
m = re_remove_add.fullmatch(subchunk)
if not m:
return False
remove = re.sub(r'^-', '', m.group('remove'), flags=re.MULTILINE)
remove = re.sub(r'\s', '', remove)
add = re.sub(r'^\+', '', m.group('add'), flags=re.MULTILINE)
add = re.sub(r'\s', '', add)
m = re_remove.fullmatch(remove)
if not m:
return False
all_functions.add(m.group('func'))
func_call = m.group('func_call')
m = re_add.fullmatch(add)
if not m:
return False
return func_call == m.group('func_call')
with open(sys.argv[1]) as f:
patch = f.read()
# Drop patch header
patch = re.sub('^.*?^---$.*?^diff', 'diff', patch,
flags=(re.MULTILINE | re.DOTALL))
# Drop patch footer
patch = re.sub(r'^-- *\n(\d+\.)+\d+\s*\Z', '', patch, flags=re.MULTILINE)
files = re.split(r'(^diff .*\n'
r'^index .*\n'
r'^--- .*\n'
r'^\+\+\+ .*)\n', patch, flags=re.MULTILINE)
assert files[0] == ''
del files[0]
subchunk_re = re.compile(r'(^[+-].*\n)+', flags=re.MULTILINE)
all_ok = True
for i in range(0, len(files), 2):
file = files[i]
patch = files[i + 1]
print_caption = True
for chunk in re.split('^@', patch, flags=re.MULTILINE):
if not all(check_subchunk(m.group(0))
for m in subchunk_re.finditer(chunk)):
if print_caption:
print(file)
print_caption = False
print(chunk)
all_ok = False
if all_ok:
print('ALL OK.\nfound functions:\n')
print('\n'.join(list(all_functions)))
run it:
# ./check2.py /work/nfs_share/patches/markus/errors/v2/\[PATCH\ v2\ 13_44\]\
qemu-option\:\ Use\ returned\ bool\ to\ check\ for\ failure\ -\ Markus\ Armbruster\
\\ -\ 2020-07-02\ 1849.eml
ALL OK.
found functions:
qemu_opt_set_bool
qemu_opts_do_parse
qemu_opt_set
qemu_opts_absorb_qdict
qemu_opt_parse
qemu_opt_rename
qemu_opts_validate
parse_option_size
qemu_opts_from_qdict_entry
opts_do_parse
functions seems to have corresponding semantics, all chunks are valid:
Reviewed-by: Vladimir Sementsov-Ogievskiy
--
Best regards,
Vladimir
Re: [PATCH v2 13/44] qemu-option: Use returned bool to check for failure
On 7/2/20 10:49 AM, Markus Armbruster wrote:
The previous commit enables conversion of
foo(..., &err);
if (err) {
...
}
to
if (!foo(..., &err)) {
...
}
for QemuOpts functions that now return true / false on success /
error. Coccinelle script:
@@
identifier fun = {opts_do_parse, parse_option_bool, parse_option_number,
parse_option_size, qemu_opt_parse, qemu_opt_rename, qemu_opt_set,
qemu_opt_set_bool, qemu_opt_set_number, qemu_opts_absorb_qdict,
qemu_opts_do_parse, qemu_opts_from_qdict_entry, qemu_opts_set,
qemu_opts_validate};
expression list args, args2;
typedef Error;
Error *err;
@@
-fun(args, &err, args2);
-if (err)
+if (!fun(args, &err, args2))
{
...
}
A few line breaks tidied up manually.
Signed-off-by: Markus Armbruster
---
Similar results to the script in 3/44.
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
[PATCH v2 13/44] qemu-option: Use returned bool to check for failure
The previous commit enables conversion of
foo(..., &err);
if (err) {
...
}
to
if (!foo(..., &err)) {
...
}
for QemuOpts functions that now return true / false on success /
error. Coccinelle script:
@@
identifier fun = {opts_do_parse, parse_option_bool, parse_option_number,
parse_option_size, qemu_opt_parse, qemu_opt_rename, qemu_opt_set,
qemu_opt_set_bool, qemu_opt_set_number, qemu_opts_absorb_qdict,
qemu_opts_do_parse, qemu_opts_from_qdict_entry, qemu_opts_set,
qemu_opts_validate};
expression list args, args2;
typedef Error;
Error *err;
@@
-fun(args, &err, args2);
-if (err)
+if (!fun(args, &err, args2))
{
...
}
A few line breaks tidied up manually.
Signed-off-by: Markus Armbruster
---
block.c | 16 ++--
block/blkdebug.c | 3 +--
block/blklogwrites.c | 3 +--
block/blkverify.c | 3 +--
block/crypto.c| 3 +--
block/curl.c | 3 +--
block/file-posix.c| 6 ++
block/file-win32.c| 6 ++
block/gluster.c | 15 +--
block/iscsi.c | 3 +--
block/nbd.c | 3 +--
block/parallels.c | 3 +--
block/qcow2.c | 3 +--
block/quorum.c| 3 +--
block/raw-format.c| 3 +--
block/replication.c | 3 +--
block/sheepdog.c | 3 +--
block/ssh.c | 3 +--
block/throttle.c | 3 +--
block/vpc.c | 3 +--
block/vvfat.c | 3 +--
block/vxhs.c | 6 ++
blockdev.c| 11 ---
chardev/char.c| 6 ++
contrib/ivshmem-server/main.c | 4 ++--
hw/net/virtio-net.c | 5 ++---
hw/smbios/smbios.c| 24
qapi/string-input-visitor.c | 3 +--
qemu-img.c| 19 +++
tpm.c | 3 +--
util/qemu-config.c| 12
util/qemu-option.c| 16 ++--
32 files changed, 71 insertions(+), 132 deletions(-)
diff --git a/block.c b/block.c
index 6dbcb7e083..8d478bdc51 100644
--- a/block.c
+++ b/block.c
@@ -1629,8 +1629,7 @@ static int bdrv_open_common(BlockDriverState *bs,
BlockBackend *file,
assert(options != NULL && bs->options != options);
opts = qemu_opts_create(&bdrv_runtime_opts, NULL, 0, &error_abort);
-qemu_opts_absorb_qdict(opts, options, &local_err);
-if (local_err) {
+if (!qemu_opts_absorb_qdict(opts, options, &local_err)) {
error_propagate(errp, local_err);
ret = -EINVAL;
goto fail_opts;
@@ -4091,8 +4090,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state,
BlockReopenQueue *queue,
/* Process generic block layer options */
opts = qemu_opts_create(&bdrv_runtime_opts, NULL, 0, &error_abort);
-qemu_opts_absorb_qdict(opts, reopen_state->options, &local_err);
-if (local_err) {
+if (!qemu_opts_absorb_qdict(opts, reopen_state->options, &local_err)) {
error_propagate(errp, local_err);
ret = -EINVAL;
goto error;
@@ -6078,8 +6076,7 @@ void bdrv_img_create(const char *filename, const char
*fmt,
/* Parse -o options */
if (options) {
-qemu_opts_do_parse(opts, options, NULL, &local_err);
-if (local_err) {
+if (!qemu_opts_do_parse(opts, options, NULL, &local_err)) {
goto out;
}
}
@@ -6092,8 +6089,8 @@ void bdrv_img_create(const char *filename, const char
*fmt,
}
if (base_filename) {
-qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, base_filename, &local_err);
-if (local_err) {
+if (!qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, base_filename,
+ &local_err)) {
error_setg(errp, "Backing file not supported for file format '%s'",
fmt);
goto out;
@@ -6101,8 +6098,7 @@ void bdrv_img_create(const char *filename, const char
*fmt,
}
if (base_fmt) {
-qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT, base_fmt, &local_err);
-if (local_err) {
+if (!qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT, base_fmt, &local_err)) {
error_setg(errp, "Backing file format not supported for file "
"format '%s'", fmt);
goto out;
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 7194bc7f06..d473dcf8c7 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -472,8 +472,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict
*options, int flags,
uint64_t align;
opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
-qemu_opts_absorb_qdict(opts, options, &local_err);
-if (local_err) {
+if (!qemu_opts_absorb_qdict(opts, options, &local_err
