On Tue, Aug 24, 2021 at 04:25:04PM +0200, Ján Tomko wrote: you can use fh.readlines() directly

On Tue, Aug 24, 2021 at 04:25:05PM +0200, Ján Tomko wrote: I know that it's preexisting but this is not recommended way how to concatenate two or more strings in python. I would use os.path.join(args.prefix, 'src/qemu/qemu_capabilities.h') instead, or even better: os.path.join(args.prefix, 'src', 'qemu', 'qemu_capabilities.h')

Enumerate over lines early to find out the position of the starting and ending markers. Then run the actual function on the lines between those markers. Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; --- scripts/group-qemu-caps.py | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/scripts/group-qemu-caps.py b/scripts/group-qemu-caps.py index bd22dd992a..e56c306d10 100755 --- a/scripts/group-qemu-caps.py +++ b/scripts/group-qemu-caps.py @@ -29,27 +29,29 @@ import sys def load_caps_flags(filename, start_regex, end_regex): capsflags = [] - game_on = False lines = [] + start = 0 + end = 0 with open(filename, "r") as fh: lines = fh.read().splitlines() - for line in lines: - if game_on: - if re.search(r'''.*/\* [0-9]+ \*/.*''', line): - continue - if re.search(r'''^\s*$''', line): - continue - match = re.search(r'''[ ]+([A-Z0-9_]+)''', line) - - if match: - capsflags.append(match[1]) - + for idx, line in enumerate(lines): if re.search(start_regex, line): - game_on = True - elif game_on and re.search(end_regex, line): - game_on = False + start = idx + elif re.search(end_regex, line): + end = idx + break + + for line in lines[start:end]: + if re.search(r'''.*/\* [0-9]+ \*/.*''', line): + continue + if re.search(r'''^\s*$''', line): + continue + match = re.search(r'''[ ]+([A-Z0-9_]+)''', line) + + if match: + capsflags.append(match[1]) return capsflags -- 2.31.1

For consistency, operate on split lines and re-add the newlines later. Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; --- scripts/group-qemu-caps.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/scripts/group-qemu-caps.py b/scripts/group-qemu-caps.py index ee33f1cf16..ca5f9b7247 100755 --- a/scripts/group-qemu-caps.py +++ b/scripts/group-qemu-caps.py @@ -58,14 +58,12 @@ def regroup_caps(check, filename, start_regex, end_regex, original = [] with open(filename, "r") as fh: - for line in fh: - original.append(line) + original = fh.read().splitlines() fixed = [] game_on = False counter = 0 for line in original: - line = line.rstrip("\n") if game_on: if re.search(r'''.*/\* [0-9]+ \*/.*''', line): continue @@ -73,8 +71,8 @@ def regroup_caps(check, filename, start_regex, end_regex, continue if counter % step == 0: if counter != 0: - fixed.append("\n") - fixed.append("%s/* %d */\n" % (counter_prefix, counter)) + fixed.append("") + fixed.append("%s/* %d */" % (counter_prefix, counter)) if not (line.find("/*") != -1 and line.find("*/") == -1): # count two-line comments as one line @@ -89,7 +87,7 @@ def regroup_caps(check, filename, start_regex, end_regex, fixed = fixed[:-1] # \n if trailing_newline: - fixed.append("\n") + fixed.append("") game_on = False @@ -99,11 +97,11 @@ def regroup_caps(check, filename, start_regex, end_regex, if flagname: line = flagname[0] + " /* %s */" % capsflags[counter - 1] - fixed.append(line + "\n") + fixed.append(line) if check: - orig = "".join(original) - new = "".join(fixed) + orig = "\n".join(original) + "\n" + new = "\n".join(fixed) + "\n" if new != orig: diff = subprocess.Popen(["diff", "-u", filename, "-"], stdin=subprocess.PIPE) @@ -117,7 +115,7 @@ def regroup_caps(check, filename, start_regex, end_regex, else: with open(filename, "w") as fh: for line in fixed: - print(line, file=fh, end='') + print(line, file=fh) return True -- 2.31.1

Get indexes of the two marker strings. Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; --- scripts/group-qemu-caps.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/scripts/group-qemu-caps.py b/scripts/group-qemu-caps.py index 2eaaf06bb4..29ab268c4c 100755 --- a/scripts/group-qemu-caps.py +++ b/scripts/group-qemu-caps.py @@ -35,19 +35,24 @@ def load_file(filename): return lines -def load_caps_flags(lines, start_regex, end_regex): - capsflags = [] +def find_markers(lines, start_marker, end_marker): start = 0 end = 0 for idx, line in enumerate(lines): - if re.search(start_regex, line): + if start_marker in line: start = idx - elif re.search(end_regex, line): + elif end_marker in line: end = idx break - for line in lines[start:end]: + return (start, end) + + +def load_caps_flags(lines): + capsflags = [] + + for line in lines: match = re.search(r'''[ ]+([A-Z0-9_]+),''', line) if match: @@ -137,9 +142,10 @@ header_path = args.prefix + 'src/qemu/qemu_capabilities.h' source_path = args.prefix + 'src/qemu/qemu_capabilities.c' header_lines = load_file(header_path) -capsflags = load_caps_flags(header_lines, - r'virQEMUCapsFlags grouping marker', - r'QEMU_CAPS_LAST \/\* this must') +(start, end) = find_markers(header_lines, + 'virQEMUCapsFlags grouping marker', + 'QEMU_CAPS_LAST /* this must') +capsflags = load_caps_flags(header_lines[start:end]) if not regroup_caps(args.check, source_path, -- 2.31.1

This makes reconstructing the fixed version of the file a bit more difficult, but lets us get rid of a cryptic bool. Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; --- scripts/group-qemu-caps.py | 63 +++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 35 deletions(-) diff --git a/scripts/group-qemu-caps.py b/scripts/group-qemu-caps.py index dce25099b8..36a7d7602f 100755 --- a/scripts/group-qemu-caps.py +++ b/scripts/group-qemu-caps.py @@ -61,48 +61,41 @@ def load_caps_flags(lines): return capsflags -def regroup_caps(original, start_regex, end_regex, +def regroup_caps(original, trailing_newline, counter_prefix, capsflags): step = 5 fixed = [] - game_on = False counter = 0 for line in original: - if game_on: - if re.search(r'''.*/\* [0-9]+ \*/.*''', line): - continue - if re.search(r'''^\s*$''', line): - continue - if counter % step == 0: - if counter != 0: - fixed.append("") - fixed.append("%s/* %d */" % (counter_prefix, counter)) - - if not (line.find("/*") != -1 and line.find("*/") == -1): - # count two-line comments as one line - counter = counter + 1 - - if re.search(start_regex, line): - game_on = True - elif game_on and re.search(end_regex, line): - if (counter - 1) % step == 0: - fixed = fixed[:-1] # /* $counter */ - if counter != 1: - fixed = fixed[:-1] # \n - - if trailing_newline: + if re.search(r'''.*/\* [0-9]+ \*/.*''', line): + continue + if re.search(r'''^\s*$''', line): + continue + if counter % step == 0: + if counter != 0: fixed.append("") + fixed.append("%s/* %d */" % (counter_prefix, counter)) - game_on = False + if not (line.find("/*") != -1 and line.find("*/") == -1): + # count two-line comments as one line + counter = counter + 1 # ensure that flag names in the .c file have the correct flag in the comment - if game_on and capsflags: + if capsflags: flagname = re.search(r'''.*".*",''', line) if flagname: line = flagname[0] + " /* %s */" % capsflags[counter - 1] fixed.append(line) + if (counter - 1) % step == 0: + fixed = fixed[:-1] # /* $counter */ + if counter != 1: + fixed = fixed[:-1] # \n + + if trailing_newline: + fixed.append("") + return fixed @@ -147,28 +140,28 @@ source_lines = load_file(source_path) 'QEMU_CAPS_LAST /* this must') capsflags = load_caps_flags(header_lines[start:end]) -header_fixed = regroup_caps(header_lines, - r'virQEMUCapsFlags grouping marker', - r'QEMU_CAPS_LAST \/\* this must', +(source_start, source_end) = find_markers(source_lines, + 'virQEMUCaps grouping marker', + ' );') + +header_fixed = regroup_caps(header_lines[start + 1:end], 1, " ", None) -source_fixed = regroup_caps(source_lines, - r'virQEMUCaps grouping marker', - r'\);', +source_fixed = regroup_caps(source_lines[source_start + 1:source_end], 0, " ", capsflags) if not check_wrapping(header_path, header_lines, - header_fixed, + header_lines[:start + 1] + header_fixed + header_lines[end:], args.check): ret = False if not check_wrapping(source_path, source_lines, - source_fixed, + source_lines[:source_start + 1] + source_fixed + source_lines[source_end:], args.check): ret = False -- 2.31.1

Use ret and exit with it directly. Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; --- scripts/group-qemu-caps.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/scripts/group-qemu-caps.py b/scripts/group-qemu-caps.py index bcda29a6ca..356d9e8d06 100755 --- a/scripts/group-qemu-caps.py +++ b/scripts/group-qemu-caps.py @@ -128,8 +128,6 @@ parser.add_argument('--prefix', default='', help='source code tree prefix') args = parser.parse_args() -errs = False - header_path = args.prefix + 'src/qemu/qemu_capabilities.h' source_path = args.prefix + 'src/qemu/qemu_capabilities.c' @@ -151,18 +149,18 @@ source_fixed = regroup_caps(source_lines[source_start + 1:source_end], " ", capsflags=capsflags) +ret = 0 + if not check_wrapping(header_path, header_lines, header_lines[:start + 1] + header_fixed + header_lines[end:], args.check): - ret = False + ret = 1 if not check_wrapping(source_path, source_lines, source_lines[:source_start + 1] + source_fixed + source_lines[source_end:], args.check): - ret = False + ret = 1 -if errs: - sys.exit(1) -sys.exit(0) +sys.exit(ret) -- 2.31.1