[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