Re: [PATCH v2 13/44] qemu-option: Use returned bool to check for failure

[Vladimir Sementsov-Ogievskiy]

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<arm...@redhat.com>

Improve reviewer script to cover indentation/spacing/wrapping changes:

#!/usr/bin/env python3

import sys
import re


re_remove_add = re.compile(r'(?P<remove>(^-.*\n)+)(?P<add>(^\+.*\n)+)',
                           flags=re.MULTILINE)
re_remove = re.compile(r'(?P<func_call>(?P<func>\w+)\(.*,&(?P<err>\w+)\));'
                       r'if\((?P=err)(!=NULL)?\)\{')
re_add = re.compile(r'if\(!(?P<func_call>.*)\)\{')
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\ 
\<arm...@redhat.com\>\ -\ 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 <vsement...@virtuozzo.com>

--
Best regards,
Vladimir