[PATCH 2/2] trace-cruncher: Modify the APIs for enable/disable multiple events

["Yordan Karadzhov (VMware)" <y.karadz@gmail.com>]

The current version of these APIs takes the list of systems and the
'list of list' of events to enable/disable in two independent arguments.
This can potentially create confusion since the user is expected to
provide two correlated lists via disjoined arguments. Here we switch to
using a single 'events' argument for these APIs that takes a dictionary
where the 'systems' are 'keys' and the lists of events from each system
are the 'value'.

Suggested-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 examples/start_tracing.py                     |  4 +-
 src/ftracepy-utils.c                          | 87 ++++++-------------
 .../tests/1_unit/test_01_ftracepy_unit.py     | 41 +++------
 3 files changed, 40 insertions(+), 92 deletions(-)

diff --git a/examples/start_tracing.py b/examples/start_tracing.py
index c9960e3..aaeb0cf 100755
--- a/examples/start_tracing.py
+++ b/examples/start_tracing.py
@@ -13,8 +13,8 @@ inst = ft.create_instance()
 
 # Enable all static events from systems "sched" and "irq".
 ft.enable_events(instance=inst,
-                 systems=['sched', 'irq'],
-                 events=[['sched_switch'],['all']])
+                 events={'sched': ['sched_switch', 'sched_waking'],
+                         'irq':   ['all']})
 
 # Print the stream of trace events. "Ctrl+c" to stop tracing.
 ft.read_trace(instance=inst)
diff --git a/src/ftracepy-utils.c b/src/ftracepy-utils.c
index 5c8bcca..d25d873 100644
--- a/src/ftracepy-utils.c
+++ b/src/ftracepy-utils.c
@@ -1161,90 +1161,59 @@ PyObject *PyFtrace_disable_event(PyObject *self, PyObject *args,
 static bool set_enable_events(PyObject *self, PyObject *args, PyObject *kwargs,
 			      bool enable)
 {
-	static char *kwlist[] = {"instance", "systems", "events", NULL};
-	PyObject *system_list = NULL, *event_list = NULL, *system_event_list;
-	const char **systems = NULL, **events = NULL;
+	static char *kwlist[] = {"events", "instance", NULL};
+	PyObject *event_dict = NULL, *py_inst = NULL;
 	struct tracefs_instance *instance;
-	PyObject *py_inst = NULL;
-	char *file = NULL;
-	int ret, s, e;
+	PyObject *py_system, *py_events;
+	const char *system, *event;
+	Py_ssize_t pos = 0;
+	int i, n;
 
 	if (!PyArg_ParseTupleAndKeywords(args,
 					 kwargs,
-					 "|OOO",
+					 "O|O",
 					 kwlist,
-					 &py_inst,
-					 &system_list,
-					 &event_list)) {
+					 &event_dict,
+					 &py_inst)) {
 		return false;
 	}
 
 	if (!get_optional_instance(py_inst, &instance))
 		return false;
 
-	if (!system_list && !event_list)
-		return event_enable_disable(instance, NULL, NULL, enable);
+	if (!PyDict_CheckExact(event_dict))
+		goto fail_with_err;
 
-	if (!system_list && event_list) {
-		if (PyUnicode_Check(event_list) &&
-		    is_all(PyUnicode_DATA(event_list))) {
-			return event_enable_disable(instance, NULL, NULL, enable);
-		} else {
-			TfsError_setstr(instance,
-					"Failed to enable events for unspecified system");
-			return false;
-		}
-	}
+	while (PyDict_Next(event_dict, &pos, &py_system, &py_events)) {
+		if (!PyUnicode_Check(py_system) ||
+		    !PyList_CheckExact(py_events))
+			goto fail_with_err;
 
-	systems = get_arg_list(system_list);
-	if (!systems) {
-		TfsError_setstr(instance, "Inconsistent \"systems\" argument.");
-		return false;
-	}
+		system = PyUnicode_DATA(py_system);
+		n = PyList_Size(py_events);
 
-	if (!event_list) {
-		for (s = 0; systems[s]; ++s) {
-			ret = event_enable_disable(instance, systems[s], NULL, enable);
-			if (ret < 0)
+		if (n == 0 || (n == 1 && is_all(str_from_list(py_events, 0)))) {
+			if (!event_enable_disable(instance, system, NULL,
+						  enable))
 				return false;
+			continue;
 		}
 
-		return true;
-	}
-
-	if (!PyList_CheckExact(event_list))
-		goto fail_with_err;
-
-	for (s = 0; systems[s]; ++s) {
-		system_event_list = PyList_GetItem(event_list, s);
-		if (!system_event_list || !PyList_CheckExact(system_event_list))
-			goto fail_with_err;
-
-		events = get_arg_list(system_event_list);
-		if (!events)
-			goto fail_with_err;
+		for (i = 0; i < n; ++i) {
+			event = str_from_list(py_events, i);
+			if (!event)
+				goto fail_with_err;
 
-		for (e = 0; events[e]; ++e) {
-			if (!event_enable_disable(instance, systems[s], events[e], enable))
-				goto fail;
+			if (!event_enable_disable(instance, system, event,
+						  enable))
+				return false;
 		}
-
-		free(events);
-		events = NULL;
 	}
 
-	free(systems);
-
 	return true;
 
  fail_with_err:
 	TfsError_setstr(instance, "Inconsistent \"events\" argument.");
-
- fail:
-	free(systems);
-	free(events);
-	free(file);
-
 	return false;
 }
 
diff --git a/tracecruncher/tests/1_unit/test_01_ftracepy_unit.py b/tracecruncher/tests/1_unit/test_01_ftracepy_unit.py
index d3e3960..b9c8fd0 100644
--- a/tracecruncher/tests/1_unit/test_01_ftracepy_unit.py
+++ b/tracecruncher/tests/1_unit/test_01_ftracepy_unit.py
@@ -270,20 +270,8 @@ class EventsTestCase(unittest.TestCase):
     def test_enable_events(self):
         inst = ft.create_instance(instance_name)
         ft.enable_events(instance=inst,
-                         events='all')
-
-        ret = ft.event_is_enabled(instance=inst,
-                                  event='all')
-        self.assertEqual(ret, '1')
-        ft.disable_events(instance=inst,
-                          events='all')
-
-        ret = ft.event_is_enabled(instance=inst,
-                                  event='all')
-        self.assertEqual(ret, '0')
-
-        ft.enable_events(instance=inst,
-                         systems=['sched', 'irq'])
+                         events={'sched': ['all'],
+                                 'irq': ['all']})
 
         ret = ft.event_is_enabled(instance=inst,
                                   system='sched',
@@ -296,7 +284,8 @@ class EventsTestCase(unittest.TestCase):
         self.assertEqual(ret, '1')
 
         ft.disable_events(instance=inst,
-                          systems=['sched', 'irq'])
+                          events={'sched': ['all'],
+                                  'irq': ['all']})
 
         ret = ft.event_is_enabled(instance=inst,
                                   system='sched',
@@ -309,9 +298,8 @@ class EventsTestCase(unittest.TestCase):
         self.assertEqual(ret, '0')
 
         ft.enable_events(instance=inst,
-                         systems=['sched', 'irq'],
-                         events=[['sched_switch', 'sched_waking'],
-                                 ['all']])
+                         events={'sched': ['sched_switch', 'sched_waking'],
+                                 'irq':   ['all']})
 
         ret = ft.event_is_enabled(instance=inst,
                                   system='sched',
@@ -334,9 +322,8 @@ class EventsTestCase(unittest.TestCase):
         self.assertEqual(ret, '1')
 
         ft.disable_events(instance=inst,
-                          systems=['sched', 'irq'],
-                          events=[['sched_switch', 'sched_waking'],
-                                  ['all']])
+                          events={'sched': ['sched_switch', 'sched_waking'],
+                                  'irq':   ['all']})
 
         ret = ft.event_is_enabled(instance=inst,
                                   system='sched',
@@ -359,21 +346,13 @@ class EventsTestCase(unittest.TestCase):
         err = 'Inconsistent \"events\" argument'
         with self.assertRaises(Exception) as context:
             ft.enable_events(instance=inst,
-                             systems=['sched'],
-                             events=['all'])
-        self.assertTrue(err in str(context.exception))
-
-        err = 'Failed to enable events for unspecified system'
-        with self.assertRaises(Exception) as context:
-            ft.enable_events(instance=inst,
-                             events=['sched_switch', 'sched_wakeup'])
+                             events='all')
         self.assertTrue(err in str(context.exception))
 
         err = 'Failed to enable/disable event'
         with self.assertRaises(Exception) as context:
             ft.enable_events(instance=inst,
-                             systems=['sched'],
-                             events=[['no_event']])
+                             events={'sched': ['no_event']})
         self.assertTrue(err in str(context.exception))
 
 
-- 
2.30.2