[PATCH 2/5] tuna: Eliminate side effects from argparse by deferring type conversions
John Kacur <[email protected]> Thu, 11 Jun 2026 16:38:29 -0400
| Newsgroups | org.kernel.vger.linux-rt-users |
|---|---|
| Message-ID | <[email protected]> |
Refactor argument parsing to remove side effects during the argparse phase by deferring type conversions until after parsing completes. This creates a clearer separation between parsing, conversion, and execution. Problems with the previous implementation: - Converter functions (threadstring_to_list, irqstring_to_list) were called via type= parameter during argparse parsing - These converters set global variables and created procfs objects before knowing what command would run - Global state (match_requested) was used to coordinate between parsing time and execution time Changes: - Removed type= parameters for threadstring_to_list and irqstring_to_list from POS and MODS dictionaries, changed to type=str with default='' - Changed sockets argument to store in 'sockets' dest instead of 'cpu_list' - Updated threadstring_to_list() to accept ps parameter and return tuple (thread_list, match_requested_flag) - Updated irqstring_to_list() to return tuple (irq_list, match_requested_flag) - Added explicit conversion block in main() after argument parsing - Eliminated match_requested global variable, now local in main() - Updated ps_show(), show_irqs(), do_ps() to accept match_requested parameter - Kept ps and irqs as module-level variables for now Benefits: - No side effects during argument parsing - Clear control flow: parse → convert → execute - match_requested is now local state instead of global - Easier to understand and maintain - Sets foundation for future refactoring of remaining globals All 6 existing unit tests pass. Assisted-by: Claude:claude-sonnet-4-5 Signed-off-by: John Kacur <[email protected]> --- tuna-cmd.py | 65 ++++++++++++++++++++++++++++++++--------------------- 1 file changed, 39 insertions(+), 26 deletions(-) diff --git a/tuna-cmd.py b/tuna-cmd.py index 54f0666fa1bb..4da6c6ea6ce5 100755 --- a/tuna-cmd.py +++ b/tuna-cmd.py @@ -79,7 +79,6 @@ except ImportError: ps = None irqs = None -match_requested = False class HelpMessageParser(argparse.ArgumentParser): def error(self, message): @@ -92,7 +91,7 @@ def gen_parser(): POS = { "cpu_list": dict(metavar='CPU-LIST', type=tuna.cpustring_to_list, help="CPU-LIST affected by commands"), - "thread_list": dict(metavar='THREAD-LIST', type=threadstring_to_list, help="THREAD-LIST affected by commands"), + "thread_list": dict(metavar='THREAD-LIST', type=str, help="THREAD-LIST affected by commands"), "filename": dict(metavar='FILENAME', type=str, help="Save kthreads sched tunables to this file"), "profilename": dict(type=str, help="Apply changes described in this file"), "run_command": dict(metavar='COMMAND', type=str, help="fork a new process and run the \"COMMAND\""), @@ -103,10 +102,10 @@ def gen_parser(): "logging": dict(dest='loglevel', metavar='LOG-LEVEL', type=get_loglevel, help="Log application details to file for given LOG-LEVEL"), "debug" : dict(action='store_true', dest='debug', help='Print DEBUG level logging details to console'), "version": dict(action='version', version='0.20', help="show version"), - "threads": dict(dest='thread_list', default=[], metavar='THREAD-LIST', type=threadstring_to_list, help="THREAD-LIST affected by commands"), - "irqs": dict(dest='irq_list', default=[], metavar='IRQ-LIST', type=irqstring_to_list, help="IRQ-LIST affect by commands"), + "threads": dict(dest='thread_list', default='', metavar='THREAD-LIST', type=str, help="THREAD-LIST affected by commands"), + "irqs": dict(dest='irq_list', default='', metavar='IRQ-LIST', type=str, help="IRQ-LIST affect by commands"), "cpus": dict(dest='cpu_list', default=[], metavar='CPU-LIST', type=tuna.cpustring_to_list, help='CPU-LIST affected by commands'), - "sockets": dict(dest='cpu_list', default=[], metavar='CPU-SOCKET-LIST', type=socketstring_to_list, help="CPU-SOCKET-LIST affected by commands"), + "sockets": dict(dest='sockets', default='', metavar='CPU-SOCKET-LIST', type=str, help="CPU-SOCKET-LIST affected by commands"), "show_sockets": dict(action='store_true', help='Show network sockets in use by threads'), "cgroups": dict(action='store_true', dest='cgroups', help='Display the processes with the type of cgroups they are in'), "spaced": dict(action='store_false', dest='compact', help='Display spaced view for cgroups'), @@ -394,9 +393,7 @@ def ps_show_thread(pid, affect_children, ps, has_ctxt_switch_info, sock_inodes, def ps_show(ps, affect_children, thread_list, cpu_list, irq_list_numbers, show_uthreads, show_kthreads, - has_ctxt_switch_info, sock_inodes, sock_inode_re, cgroups, compact): - - global match_requested + has_ctxt_switch_info, sock_inodes, sock_inode_re, cgroups, compact, match_requested): ps_list = [] for pid in list(ps.keys()): @@ -467,7 +464,7 @@ def load_sockets(): def do_ps(thread_list, cpu_list, irq_list, show_uthreads, show_kthreads, - affect_children, show_sockets, cgroups, compact): + affect_children, show_sockets, cgroups, compact, match_requested): ps = procfs.pidstats() if affect_children: ps.reload_threads() @@ -484,7 +481,7 @@ def do_ps(thread_list, cpu_list, irq_list, show_uthreads, show_kthreads, ps_show_header(has_ctxt_switch_info, cgroups) ps_show(ps, affect_children, thread_list, cpu_list, irq_list, show_uthreads, show_kthreads, - has_ctxt_switch_info, sock_inodes, sock_inode_re, cgroups, compact) + has_ctxt_switch_info, sock_inodes, sock_inode_re, cgroups, compact, match_requested) except IOError: # 'tuna -P | head' for instance pass @@ -507,9 +504,8 @@ def find_drivers_by_users(users): return drivers -def show_irqs(irq_list, cpu_list): +def show_irqs(irq_list, cpu_list, match_requested): global irqs - global match_requested if not irqs: irqs = procfs.interrupts() @@ -554,30 +550,29 @@ def do_list_op(op, current_list, op_list): return list(set(current_list) - set(op_list)) return list(set(op_list)) -def threadstring_to_list(threadstr): - global ps - global match_requested - if threadstr: - match_requested = True +def threadstring_to_list(threadstr, ps=None): + match_requested = bool(threadstr) thread_list = [] + if not threadstr: + return thread_list, match_requested thread_strings = list(set(threadstr.split(','))) for s in thread_strings: if s.isdigit(): thread_list.append(int(s)) else: - ps = procfs.pidstats() + if ps is None: + ps = procfs.pidstats() try: thread_list += ps.find_by_regex(re.compile(fnmatch.translate(s))) except re.error: thread_list += ps.find_by_name(s) - return thread_list + return thread_list, match_requested def irqstring_to_list(irqstr): - - global match_requested - if irqstr: - match_requested = True + match_requested = bool(irqstr) irq_list = [] + if not irqstr: + return irq_list, match_requested irq_strings = list(set(irqstr.split(','))) for s in irq_strings: if s.isdigit(): @@ -586,7 +581,7 @@ def irqstring_to_list(irqstr): # find_by_user_regex returns a list of strings corresponding to irq number irq_list_str = procfs.interrupts().find_by_user_regex(re.compile(fnmatch.translate(s))) irq_list += [int(i) for i in irq_list_str if i.isdigit()] - return irq_list + return irq_list, match_requested def socketstring_to_list(socketstr): cpu_list = [] @@ -684,6 +679,24 @@ def main(): print("Valid log levels: NOTSET, DEBUG, INFO, WARNING, ERROR") print("Log levels may be specified numerically (0-4)\n") + # Convert string arguments to lists after parsing + match_requested = False + + # Convert thread_list from string to list + if 'thread_list' in vars(args) and args.thread_list: + if isinstance(args.thread_list, str): + args.thread_list, match_requested = threadstring_to_list(args.thread_list, ps) + + # Convert irq_list from string to list + if 'irq_list' in vars(args) and args.irq_list: + if isinstance(args.irq_list, str): + args.irq_list, irq_match_requested = irqstring_to_list(args.irq_list) + match_requested = match_requested or irq_match_requested + + # Convert socket strings to cpu lists + if hasattr(args, 'sockets') and args.sockets: + args.cpu_list = socketstring_to_list(args.sockets) + if args.command == 'cpu_power': if not cpw.have_cpupower: print(f"Error: libcpupower bindings are not detected; please install libcpupower bindings from at least kernel {cpw.cpupower_required_kernel}.", file=sys.stderr) @@ -738,10 +751,10 @@ def main(): elif args.command in ['show_threads']: do_ps(args.thread_list, args.cpu_list, args.irq_list, args.uthreads, - args.kthreads, args.affect_children, args.show_sockets if "show_sockets" in args else None, args.cgroups, args.compact) + args.kthreads, args.affect_children, args.show_sockets if "show_sockets" in args else None, args.cgroups, args.compact, match_requested) elif args.command in ['show_irqs']: - show_irqs(args.irq_list, args.cpu_list) + show_irqs(args.irq_list, args.cpu_list, match_requested) elif args.command in ['move', 'm', 'spread', 'x']: spread = args.command in ['spread', 'x'] -- 2.54.0