[v2,8/8] trace-cmd: Move tracecmd_stack_tracer_status() function to libtracecmd
diff mbox series

Message ID 20190814084712.28188-9-tz.stoyanov@gmail.com
State Superseded
Delegated to: Steven Rostedt
Headers show
Series
  • Separate trace-cmd and libtracecmd code
Related show

Commit Message

Tzvetomir Stoyanov (VMware) Aug. 14, 2019, 8:47 a.m. UTC
tracecmd_stack_tracer_status() function reads the stack tracer status
from the proc file system. It does not depend on trace-cmd context and
can be used standalone. The function is moved from trace-cmd application
into libtracecmd.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 lib/trace-cmd/trace-util.c | 49 +++++++++++++++++++++++++++++++++
 tracecmd/trace-stack.c     | 56 ++------------------------------------
 2 files changed, 51 insertions(+), 54 deletions(-)

Comments

Steven Rostedt Aug. 28, 2019, 8:21 p.m. UTC | #1
On Wed, 14 Aug 2019 11:47:08 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> tracecmd_stack_tracer_status() function reads the stack tracer status
> from the proc file system. It does not depend on trace-cmd context and
> can be used standalone. The function is moved from trace-cmd application
> into libtracecmd.
> 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  lib/trace-cmd/trace-util.c | 49 +++++++++++++++++++++++++++++++++
>  tracecmd/trace-stack.c     | 56 ++------------------------------------
>  2 files changed, 51 insertions(+), 54 deletions(-)
> 
> diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c
> index 8c1a0a0..d16a018 100644
> --- a/lib/trace-cmd/trace-util.c
> +++ b/lib/trace-cmd/trace-util.c
> @@ -25,6 +25,7 @@
>  #define LOCAL_PLUGIN_DIR ".trace-cmd/plugins"
>  #define TRACEFS_PATH "/sys/kernel/tracing"
>  #define DEBUGFS_PATH "/sys/kernel/debug"
> +#define PROC_STACK_FILE "/proc/sys/kernel/stack_tracer_enabled"
>  
>  int tracecmd_disable_sys_plugins;
>  int tracecmd_disable_plugins;
> @@ -1787,3 +1788,51 @@ int trace_set_log_file(char *logfile)
>  	return 0;
>  }
>  
> +/**
> + * tracecmd_stack_tracer_status - Check stack trace status
> + * @status: Returned stack trace status:
> + *		0 - not configured, disabled
> + *		non 0 - enabled
> + *
> + * Returns -1 in case of an error, 0 if file does not exist
> + *  (stack tracer not enabled) or 1 on successful completion.

  "(stack tracer not configured in kernel)"

Saying "enabled" is ambiguous.

> + */
> +int tracecmd_stack_tracer_status(int *status)

If we are making this a library function, shouldn't it be declared in
the trace-cmd.h file?

-- Steve
Tzvetomir Stoyanov (VMware) Sept. 3, 2019, 12:24 p.m. UTC | #2
Hi Steven,

On Wed, Aug 28, 2019 at 11:21 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
...
> > +/**
> > + * tracecmd_stack_tracer_status - Check stack trace status
> > + * @status: Returned stack trace status:
> > + *           0 - not configured, disabled
> > + *           non 0 - enabled
> > + *
> > + * Returns -1 in case of an error, 0 if file does not exist
> > + *  (stack tracer not enabled) or 1 on successful completion.
>
>   "(stack tracer not configured in kernel)"
>
> Saying "enabled" is ambiguous.
>
> > + */
> > +int tracecmd_stack_tracer_status(int *status)
>
> If we are making this a library function, shouldn't it be declared in
> the trace-cmd.h file?
>

It is already part of trace-cmd.h, that was one of the problems - it was defined
as library API, but implemented in the trace-cmd application. This patch moves
the implementation in the library, the declaration in trace-cmd.h
remains the same.

> -- Steve

Patch
diff mbox series

diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c
index 8c1a0a0..d16a018 100644
--- a/lib/trace-cmd/trace-util.c
+++ b/lib/trace-cmd/trace-util.c
@@ -25,6 +25,7 @@ 
 #define LOCAL_PLUGIN_DIR ".trace-cmd/plugins"
 #define TRACEFS_PATH "/sys/kernel/tracing"
 #define DEBUGFS_PATH "/sys/kernel/debug"
+#define PROC_STACK_FILE "/proc/sys/kernel/stack_tracer_enabled"
 
 int tracecmd_disable_sys_plugins;
 int tracecmd_disable_plugins;
@@ -1787,3 +1788,51 @@  int trace_set_log_file(char *logfile)
 	return 0;
 }
 
+/**
+ * tracecmd_stack_tracer_status - Check stack trace status
+ * @status: Returned stack trace status:
+ *		0 - not configured, disabled
+ *		non 0 - enabled
+ *
+ * Returns -1 in case of an error, 0 if file does not exist
+ *  (stack tracer not enabled) or 1 on successful completion.
+ */
+int tracecmd_stack_tracer_status(int *status)
+{
+	struct stat stat_buf;
+	char buf[64];
+	long num;
+	int fd;
+	int n;
+
+	if (stat(PROC_STACK_FILE, &stat_buf) < 0) {
+		/* stack tracer not configured on running kernel */
+		*status = 0; /* not configured means disabled */
+		return 0;
+	}
+
+	fd = open(PROC_STACK_FILE, O_RDONLY);
+
+	if (fd < 0)
+		return -1;
+
+	n = read(fd, buf, sizeof(buf));
+	close(fd);
+
+	if (n <= 0)
+		return -1;
+
+	if (n >= sizeof(buf))
+		return -1;
+
+	buf[n] = 0;
+	errno = 0;
+	num = strtol(buf, NULL, 10);
+
+	/* Check for various possible errors */
+	if (num > INT_MAX || num < INT_MIN || (!num && errno))
+		return -1;
+
+	*status = num;
+	return 1; /* full success */
+}
diff --git a/tracecmd/trace-stack.c b/tracecmd/trace-stack.c
index 34b3b58..bb002c0 100644
--- a/tracecmd/trace-stack.c
+++ b/tracecmd/trace-stack.c
@@ -36,58 +36,6 @@  static void test_available(void)
 		die("stack tracer not configured on running kernel");
 }
 
-/*
- * Returns:
- *   -1 - Something went wrong
- *    0 - File does not exist (stack tracer not enabled)
- *    1 - Success
- */
-static int read_proc(int *status)
-{
-	struct stat stat_buf;
-	char buf[64];
-	long num;
-	int fd;
-	int n;
-
-	if (stat(PROC_FILE, &stat_buf) < 0) {
-		/* stack tracer not configured on running kernel */
-		*status = 0; /* not configured means disabled */
-		return 0;
-	}
-
-	fd = open(PROC_FILE, O_RDONLY);
-
-	if (fd < 0)
-		return -1;
-
-	n = read(fd, buf, sizeof(buf));
-	close(fd);
-
-	if (n <= 0)
-		return -1;
-
-	if (n >= sizeof(buf))
-		return -1;
-
-	buf[n] = 0;
-	errno = 0;
-	num = strtol(buf, NULL, 10);
-
-	/* Check for various possible errors */
-	if (num > INT_MAX || num < INT_MIN || (!num && errno))
-		return -1;
-
-	*status = num;
-	return 1; /* full success */
-}
-
-/* Public wrapper of read_proc() */
-int tracecmd_stack_tracer_status(int *status)
-{
-	return read_proc(status);
-}
-
 /* NOTE: this implementation only accepts new_status in the range [0..9]. */
 static void change_stack_tracer_status(unsigned new_status)
 {
@@ -102,7 +50,7 @@  static void change_stack_tracer_status(unsigned new_status)
 		return;
 	}
 
-	ret = read_proc(&status);
+	ret = tracecmd_stack_tracer_status(&status);
 	if (ret < 0)
 		die("error reading %s", PROC_FILE);
 
@@ -160,7 +108,7 @@  static void read_trace(void)
 	size_t n;
 	int r;
 
-	if (read_proc(&status) <= 0)
+	if (tracecmd_stack_tracer_status(&status) <= 0)
 		die("Invalid stack tracer state");
 
 	if (status > 0)