[PATCH 0/5] trace-cruncher: Improve error messages

[PATCH 0/5] trace-cruncher: Improve error messages

[Yordan Karadzhov (VMware)]

Various small fixes aiming to make the error messages, printed by
trace-cruncher, more useful.

Yordan Karadzhov (VMware) (5):
  trace-cruncher: Error on fail to destroy
  trace-cruncher: Proper type name for the python objects
  trace-cruncher: Correct misleading error message
  trace-cruncher: Proper labeling of the libtracefs errors
  trace-cruncher: Proper labeling of non libtracefs errors

 src/common.h         | 11 +++++++----
 src/ftracepy-utils.c | 26 ++++++++++++++------------
 src/ftracepy-utils.h |  2 +-
 3 files changed, 22 insertions(+), 17 deletions(-)

-- 
2.30.2

[PATCH 1/5] trace-cruncher: Error on fail to destroy

[Yordan Karadzhov (VMware)]

Always print an error message in the case when the C object,
being wrapped by the Python type failed to destroy.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 src/common.h         | 7 +++++--
 src/ftracepy-utils.c | 4 ++--
 src/ftracepy-utils.h | 2 +-
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/common.h b/src/common.h
index abfda1f..697b327 100644
--- a/src/common.h
+++ b/src/common.h
@@ -54,8 +54,9 @@ static inline void no_free(void *ptr)
 
 #define NO_FREE		no_free
 
-static inline void no_destroy(void *ptr)
+static inline int no_destroy(void *ptr)
 {
+	return 0;
 }
 
 #define NO_DESTROY	no_destroy
@@ -101,7 +102,9 @@ static int py_type##_init(py_type *self, PyObject *args, PyObject *kwargs)	\
 static void py_type##_dealloc(py_type *self)					\
 {										\
 	if (self->destroy)							\
-		obj_destroy(self->ptrObj);					\
+		if (obj_destroy(self->ptrObj) < 0)				\
+			printf("fracepy_error: object '%s' failed to destroy\n",\
+			       c_type);						\
 	ptr_free(self->ptrObj);							\
 	Py_TYPE(self)->tp_free(self);						\
 }										\
diff --git a/src/ftracepy-utils.c b/src/ftracepy-utils.c
index ca7f914..e49132d 100644
--- a/src/ftracepy-utils.c
+++ b/src/ftracepy-utils.c
@@ -1757,9 +1757,9 @@ PyObject *PyKprobe_probe(PyKprobe *self)
 	return PyUnicode_FromString(self->ptrObj->probe);
 }
 
-void ftracepy_kprobe_destroy(struct ftracepy_kprobe *kp)
+int ftracepy_kprobe_destroy(struct ftracepy_kprobe *kp)
 {
-	unregister_kprobe(kp->event);
+	return tracefs_kprobe_clear_probe(TC_SYS, kp-> event, true);
 }
 
 void ftracepy_kprobe_free(struct ftracepy_kprobe *kp)
diff --git a/src/ftracepy-utils.h b/src/ftracepy-utils.h
index 9f4c4ba..a6133cf 100644
--- a/src/ftracepy-utils.h
+++ b/src/ftracepy-utils.h
@@ -26,7 +26,7 @@ C_OBJECT_WRAPPER_DECLARE(tracefs_instance, PyTfsInstance)
 
 struct ftracepy_kprobe;
 
-void ftracepy_kprobe_destroy(struct ftracepy_kprobe *kp);
+int ftracepy_kprobe_destroy(struct ftracepy_kprobe *kp);
 
 void ftracepy_kprobe_free(struct ftracepy_kprobe *kp);
 
-- 
2.30.2

[PATCH 2/5] trace-cruncher: Proper type name for the python objects

[Yordan Karadzhov (VMware)]

This name gets printed as part of the error messages. It makes
no sense to have it include the 'C' type since the developer
that will see the error message will have no idea about the 'C'
internals of the Python module.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 src/common.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/common.h b/src/common.h
index 697b327..b4c66b7 100644
--- a/src/common.h
+++ b/src/common.h
@@ -63,7 +63,7 @@ static inline int no_destroy(void *ptr)
 
 #define STR(x) #x
 
-#define MAKE_TYPE_STR(x) STR(trace.x)
+#define MAKE_TYPE_STR(x) STR(ftracepy.x)
 
 #define MAKE_DIC_STR(x) STR(libtrace x object)
 
@@ -84,7 +84,7 @@ bool py_type##_Check(PyObject *obj);						\
 
 #define  C_OBJECT_WRAPPER(c_type, py_type, obj_destroy, ptr_free)		\
 static PyTypeObject py_type##Type = {						\
-	PyVarObject_HEAD_INIT(NULL, 0) MAKE_TYPE_STR(c_type)			\
+	PyVarObject_HEAD_INIT(NULL, 0) MAKE_TYPE_STR(py_type)			\
 };										\
 PyObject *py_type##_New(struct c_type *c_ptr)					\
 {										\
-- 
2.30.2

[PATCH 3/5] trace-cruncher: Correct misleading error message

[Yordan Karadzhov (VMware)]

Indeed, the callback function can be included from a plugin, but this
is an option. In the default case it will be searched in '__main__'.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 src/ftracepy-utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/ftracepy-utils.c b/src/ftracepy-utils.c
index e49132d..b0f5e0e 100644
--- a/src/ftracepy-utils.c
+++ b/src/ftracepy-utils.c
@@ -2039,7 +2039,7 @@ static PyObject *get_callback_func(const char *plugin_name, const char * py_call
 	py_func = PyObject_GetAttrString(py_module, py_callback);
 	if (!py_func || !PyCallable_Check(py_func)) {
 		TfsError_fmt(NULL,
-			     "Failed to import callback from plugin \'%s\'",
+			     "Failed to import callback from \'%s\'",
 			     plugin_name);
 		return NULL;
 	}
-- 
2.30.2

[PATCH 4/5] trace-cruncher: Proper labeling of the libtracefs errors

[Yordan Karadzhov (VMware)]

'TEP_ERROR' is supposed to be used with errors coming from libtraceevent,
not from libtracefs.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 src/ftracepy-utils.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/ftracepy-utils.c b/src/ftracepy-utils.c
index b0f5e0e..1a8a136 100644
--- a/src/ftracepy-utils.c
+++ b/src/ftracepy-utils.c
@@ -68,14 +68,14 @@ static void TfsError_fmt(struct tracefs_instance *instance,
 		vasprintf(&tc_err_log, fmt, args);
 		va_end(args);
 
-		PyErr_Format(TEP_ERROR, "%s\ntfs_error: %s",
+		PyErr_Format(TFS_ERROR, "%s\ntfs_error: %s",
 			     tc_err_log, tfs_err_log);
 
 		tfs_clear_error_log(instance);
 		free(tfs_err_log);
 		free(tc_err_log);
 	} else {
-		PyErr_FormatV(TEP_ERROR, fmt, args);
+		PyErr_FormatV(TFS_ERROR, fmt, args);
 		va_end(args);
 	}
 }
@@ -86,11 +86,11 @@ static void TfsError_setstr(struct tracefs_instance *instance,
 	char *tfs_err_log = tfs_error_log(instance, NULL);
 
 	if (tfs_err_log) {
-		PyErr_Format(TEP_ERROR, "%s\ntfs_error: %s", msg, tfs_err_log);
+		PyErr_Format(TFS_ERROR, "%s\ntfs_error: %s", msg, tfs_err_log);
 		tfs_clear_error_log(instance);
 		free(tfs_err_log);
 	} else {
-		PyErr_SetString(TEP_ERROR, msg);
+		PyErr_SetString(TFS_ERROR, msg);
 	}
 }
 
-- 
2.30.2

[PATCH 5/5] trace-cruncher: Proper labeling of non libtracefs errors

[Yordan Karadzhov (VMware)]

Those particular errors are internal for trace-cruncher and have
nothing to do with libtracefs. The labels must indicate this.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 src/ftracepy-utils.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/ftracepy-utils.c b/src/ftracepy-utils.c
index 1a8a136..d8115f6 100644
--- a/src/ftracepy-utils.c
+++ b/src/ftracepy-utils.c
@@ -2031,14 +2031,15 @@ static PyObject *get_callback_func(const char *plugin_name, const char * py_call
 	py_name = PyUnicode_FromString(plugin_name);
 	py_module = PyImport_Import(py_name);
 	if (!py_module) {
-		TfsError_fmt(NULL, "Failed to import plugin \'%s\'",
+		PyErr_Format(TRACECRUNCHER_ERROR,
+			     "Failed to import plugin \'%s\'",
 			     plugin_name);
 		return NULL;
 	}
 
 	py_func = PyObject_GetAttrString(py_module, py_callback);
 	if (!py_func || !PyCallable_Check(py_func)) {
-		TfsError_fmt(NULL,
+		PyErr_Format(TRACECRUNCHER_ERROR,
 			     "Failed to import callback from \'%s\'",
 			     plugin_name);
 		return NULL;
@@ -2169,7 +2170,7 @@ PyObject *PyFtrace_trace_shell_process(PyObject *self, PyObject *args,
 
 	pid = fork();
 	if (pid < 0) {
-		TfsError_setstr(instance, "Failed to fork");
+		PyErr_SetString(TRACECRUNCHER_ERROR, "Failed to fork");
 		return NULL;
 	}
 
@@ -2215,7 +2216,8 @@ PyObject *PyFtrace_trace_process(PyObject *self, PyObject *args,
 		return NULL;
 
 	if (!PyList_CheckExact(py_argv)) {
-		TfsError_setstr(instance, "Failed to parse \'argv\' list");
+		PyErr_SetString(TRACECRUNCHER_ERROR,
+				"Failed to parse \'argv\' list");
 		return NULL;
 	}
 
@@ -2223,7 +2225,7 @@ PyObject *PyFtrace_trace_process(PyObject *self, PyObject *args,
 
 	pid = fork();
 	if (pid < 0) {
-		TfsError_setstr(instance, "Failed to fork");
+		PyErr_SetString(TRACECRUNCHER_ERROR, "Failed to fork");
 		return NULL;
 	}
 
-- 
2.30.2