[libvirt PATCH 0/6] scripts: group-qemu-caps: improve readability
Even though it still stays a Perl script at heart. Ján Tomko (6): scripts: group-qemu-caps: use read scripts: group-qemu-caps: remove cryptic bool scripts: group-qemu-caps: remove unecessary regex scripts: group-qemu-caps: separate file loading scripts: group-qemu-caps: regroup_caps: operate on split lines scripts: group-qemu-caps: separate file operations from the check scripts/group-qemu-caps.py | 120 +++++++++++++++++++++---------------- 1 file changed, 68 insertions(+), 52 deletions(-) -- 2.45.2
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- scripts/group-qemu-caps.py | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/scripts/group-qemu-caps.py b/scripts/group-qemu-caps.py index 8a899a76c2..ec10f24384 100755 --- a/scripts/group-qemu-caps.py +++ b/scripts/group-qemu-caps.py @@ -30,24 +30,26 @@ import sys def load_caps_flags(filename, start_regex, end_regex): capsflags = [] game_on = False + lines = [] with open(filename, "r") as fh: - for line in fh: - line = line.rstrip("\n") - 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]) - - if re.search(start_regex, line): - game_on = True - elif game_on and re.search(end_regex, line): - game_on = False + 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]) + + if re.search(start_regex, line): + game_on = True + elif game_on and re.search(end_regex, line): + game_on = False return capsflags -- 2.45.2
On Wed, Jul 31, 2024 at 03:20:47PM +0200, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- scripts/group-qemu-caps.py | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-)
diff --git a/scripts/group-qemu-caps.py b/scripts/group-qemu-caps.py index 8a899a76c2..ec10f24384 100755 --- a/scripts/group-qemu-caps.py +++ b/scripts/group-qemu-caps.py @@ -30,24 +30,26 @@ import sys def load_caps_flags(filename, start_regex, end_regex): capsflags = [] game_on = False + lines = []
with open(filename, "r") as fh: - for line in fh: - line = line.rstrip("\n") - 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]) - - if re.search(start_regex, line): - game_on = True - elif game_on and re.search(end_regex, line): - game_on = False + lines = fh.read().splitlines()
What's the point? This way you have to read the whole file into the memory. If you keep it as is and just break on `game_on = False` you skip reading the rest of the file at least. You can even do: for line in open(filename, 'r'):
+ + 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]) + + if re.search(start_regex, line): + game_on = True + elif game_on and re.search(end_regex, line): + game_on = False
return capsflags
-- 2.45.2
Instead, iterate over (a part of) the array twice: * once to find where the caps start and end * once only over the lines with caps Signed-off-by: Ján Tomko <jtomko@redhat.com> --- 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 ec10f24384..49b64c93fc 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.45.2
We're only looking at a simple string match. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- scripts/group-qemu-caps.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts/group-qemu-caps.py b/scripts/group-qemu-caps.py index 49b64c93fc..3581d1545a 100755 --- a/scripts/group-qemu-caps.py +++ b/scripts/group-qemu-caps.py @@ -27,7 +27,7 @@ import subprocess import sys -def load_caps_flags(filename, start_regex, end_regex): +def load_caps_flags(filename, start_str, end_str): capsflags = [] lines = [] start = 0 @@ -37,9 +37,9 @@ def load_caps_flags(filename, start_regex, end_regex): lines = fh.read().splitlines() for idx, line in enumerate(lines): - if re.search(start_regex, line): + if start_str in line: start = idx - elif re.search(end_regex, line): + elif end_str in line: end = idx break @@ -136,8 +136,8 @@ args = parser.parse_args() errs = False capsflags = load_caps_flags(args.prefix + 'src/qemu/qemu_capabilities.h', - r'virQEMUCapsFlags grouping marker', - r'QEMU_CAPS_LAST \/\* this must') + 'virQEMUCapsFlags grouping marker', + 'QEMU_CAPS_LAST /* this must') if not regroup_caps(args.check, args.prefix + 'src/qemu/qemu_capabilities.c', -- 2.45.2
Separate file loading from its parsing. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- scripts/group-qemu-caps.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/scripts/group-qemu-caps.py b/scripts/group-qemu-caps.py index 3581d1545a..a43cd1de45 100755 --- a/scripts/group-qemu-caps.py +++ b/scripts/group-qemu-caps.py @@ -27,8 +27,7 @@ import subprocess import sys -def load_caps_flags(filename, start_str, end_str): - capsflags = [] +def load_file(filename, start_str, end_str): lines = [] start = 0 end = 0 @@ -43,7 +42,13 @@ def load_caps_flags(filename, start_str, end_str): end = idx break - for line in lines[start:end]: + return (lines, start, end) + + +def load_caps_flags(lines): + capsflags = [] + + for line in lines: if re.search(r'''.*/\* [0-9]+ \*/.*''', line): continue if re.search(r'''^\s*$''', line): @@ -135,9 +140,10 @@ args = parser.parse_args() errs = False -capsflags = load_caps_flags(args.prefix + 'src/qemu/qemu_capabilities.h', - 'virQEMUCapsFlags grouping marker', - 'QEMU_CAPS_LAST /* this must') +(header_lines, start, end) = load_file(args.prefix + 'src/qemu/qemu_capabilities.h', + 'virQEMUCapsFlags grouping marker', + 'QEMU_CAPS_LAST /* this must') +capsflags = load_caps_flags(header_lines[start:end]) if not regroup_caps(args.check, args.prefix + 'src/qemu/qemu_capabilities.c', -- 2.45.2
This removes some newline handling at the cost of adding it elsewhere. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- 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 a43cd1de45..34823931df 100755 --- a/scripts/group-qemu-caps.py +++ b/scripts/group-qemu-caps.py @@ -67,14 +67,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 @@ -82,8 +80,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 @@ -98,7 +96,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 @@ -108,11 +106,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) @@ -126,7 +124,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.45.2
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- scripts/group-qemu-caps.py | 50 ++++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/scripts/group-qemu-caps.py b/scripts/group-qemu-caps.py index 34823931df..e4161f5bb5 100755 --- a/scripts/group-qemu-caps.py +++ b/scripts/group-qemu-caps.py @@ -61,14 +61,9 @@ def load_caps_flags(lines): return capsflags -def regroup_caps(check, filename, start_regex, end_regex, +def regroup_caps(original, start_regex, end_regex, trailing_newline, counter_prefix, capsflags): step = 5 - - original = [] - with open(filename, "r") as fh: - original = fh.read().splitlines() - fixed = [] game_on = False counter = 0 @@ -107,7 +102,10 @@ def regroup_caps(check, filename, start_regex, end_regex, line = flagname[0] + " /* %s */" % capsflags[counter - 1] fixed.append(line) + return fixed + +def check(check, filename, original, fixed): if check: orig = "\n".join(original) + "\n" new = "\n".join(fixed) + "\n" @@ -143,22 +141,32 @@ errs = False 'QEMU_CAPS_LAST /* this must') capsflags = load_caps_flags(header_lines[start:end]) -if not regroup_caps(args.check, - args.prefix + 'src/qemu/qemu_capabilities.c', - r'virQEMUCaps grouping marker', - r'\);', - 0, - " ", - capsflags): +(source_lines, source_start, source_end) = load_file(args.prefix + 'src/qemu/qemu_capabilities.c', + r'virQEMUCaps grouping marker', + r'\);') +fixed_source = regroup_caps(source_lines, + r'virQEMUCaps grouping marker', + r'\);', + 0, + " ", + capsflags) + +fixed_header = regroup_caps(header_lines, + r'virQEMUCapsFlags grouping marker', + r'QEMU_CAPS_LAST \/\* this must', + 1, + " ", + None) + +if not check(args.check, + args.prefix + 'src/qemu/qemu_capabilities.c', + source_lines, + fixed_source): errs = True - -if not regroup_caps(args.check, - args.prefix + 'src/qemu/qemu_capabilities.h', - r'virQEMUCapsFlags grouping marker', - r'QEMU_CAPS_LAST \/\* this must', - 1, - " ", - None): +if not check(args.check, + args.prefix + 'src/qemu/qemu_capabilities.h', + header_lines, + fixed_header): errs = True if errs: -- 2.45.2
participants (2)
-
Ján Tomko -
Martin Kletzander