diff mbox series

[v2,8/8] trace-cmd: Add new function to set log level

Message ID 20210507095333.1080910-9-tz.stoyanov@gmail.com (mailing list archive)
State Superseded
Headers show
Series Changes to trace-cmd logs | expand

Commit Message

Tzvetomir Stoyanov (VMware) May 7, 2021, 9:53 a.m. UTC
Introduce a new trace-cmd internal function to set the application log
level.
 trace_set_verbose()
The log level can be set using string with log id or log name.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 tracecmd/include/trace-local.h |  2 ++
 tracecmd/trace-cmd.c           | 41 ++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

Comments

Steven Rostedt May 13, 2021, 9:36 p.m. UTC | #1
On Fri,  7 May 2021 12:53:33 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> Introduce a new trace-cmd internal function to set the application log
> level.
>  trace_set_verbose()
> The log level can be set using string with log id or log name.
> 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  tracecmd/include/trace-local.h |  2 ++
>  tracecmd/trace-cmd.c           | 41 ++++++++++++++++++++++++++++++++++
>  2 files changed, 43 insertions(+)
> 
> diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h
> index b3997d00..8da07c83 100644
> --- a/tracecmd/include/trace-local.h
> +++ b/tracecmd/include/trace-local.h
> @@ -39,6 +39,8 @@ void usage(char **argv);
>  extern int silence_warnings;
>  extern int show_status;
>  
> +int trace_set_verbose(char *level);
> +
>  struct pid_record_data {
>  	int			pid;
>  	int			brass[2];
> diff --git a/tracecmd/trace-cmd.c b/tracecmd/trace-cmd.c
> index 60cd3ea1..4e4a73af 100644
> --- a/tracecmd/trace-cmd.c
> +++ b/tracecmd/trace-cmd.c
> @@ -45,6 +45,47 @@ void *malloc_or_die(unsigned int size)
>  	return data;
>  }
>  
> +static struct trace_log_severity {
> +	int		id;
> +	const char	*name;
> +} log_severtity[] = {

Is there a reason for the extra 't' in severity?

> +	{ .id = TEP_LOG_NONE, .name = "none" },
> +	{ .id = TEP_LOG_CRITICAL, .name = "crit" },
> +	{ .id = TEP_LOG_ERROR, .name = "err" },
> +	{ .id = TEP_LOG_WARNING, .name = "warn" },
> +	{ .id = TEP_LOG_INFO, .name = "info" },
> +	{ .id = TEP_LOG_DEBUG, .name = "debug" },
> +	{ .id = TEP_LOG_ALL, .name = "all" },
> +};
> +
> +int trace_set_verbose(char *level)
> +{
> +	int id;
> +
> +	if (!level)
> +		return -1;
> +
> +	if (isdigit(level[0])) {
> +		id = atoi(level);
> +		if (id >= TEP_LOG_NONE && id <= TEP_LOG_ALL) {
> +			tracecmd_set_loglevel(id);
> +			return 0;
> +		}
> +	} else {
> +		int size = sizeof(log_severtity) / sizeof(log_severtity[0]);

This can be shortened to:

		int size = ARRAY_SIZE(log_severity);

Or at least we need to copy that macro to something that trace-cmd core has
access to (see lib/trace-cmd/include/private/trace-cmd-private.h)


> +		int i;
> +
> +		for (i = 0; i < size; i++) {
> +			if (strlen(level) >= strlen(log_severtity[i].name) &&

Why the strlen test? It's OK to pass in "debugignorme"? What's the use case
for this?

Anyway, I applied the first two patches of this series.

-- Steve


> +			    !strncmp(level, log_severtity[i].name, strlen(log_severtity[i].name))) {
> +				tracecmd_set_loglevel(log_severtity[i].id);
> +				return 0;
> +			}
> +		}
> +	}
> +
> +	return -1;
> +}
>  
>  /**
>   * struct command
Tzvetomir Stoyanov (VMware) May 14, 2021, 2:53 a.m. UTC | #2
On Fri, May 14, 2021 at 12:36 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Fri,  7 May 2021 12:53:33 +0300
> "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
>
> > Introduce a new trace-cmd internal function to set the application log
> > level.
> >  trace_set_verbose()
> > The log level can be set using string with log id or log name.
> >
> > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> > ---
> >  tracecmd/include/trace-local.h |  2 ++
> >  tracecmd/trace-cmd.c           | 41 ++++++++++++++++++++++++++++++++++
> >  2 files changed, 43 insertions(+)
> >
> > diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h
> > index b3997d00..8da07c83 100644
> > --- a/tracecmd/include/trace-local.h
> > +++ b/tracecmd/include/trace-local.h
> > @@ -39,6 +39,8 @@ void usage(char **argv);
> >  extern int silence_warnings;
> >  extern int show_status;
> >
> > +int trace_set_verbose(char *level);
> > +
> >  struct pid_record_data {
> >       int                     pid;
> >       int                     brass[2];
> > diff --git a/tracecmd/trace-cmd.c b/tracecmd/trace-cmd.c
> > index 60cd3ea1..4e4a73af 100644
> > --- a/tracecmd/trace-cmd.c
> > +++ b/tracecmd/trace-cmd.c
> > @@ -45,6 +45,47 @@ void *malloc_or_die(unsigned int size)
> >       return data;
> >  }
> >
> > +static struct trace_log_severity {
> > +     int             id;
> > +     const char      *name;
> > +} log_severtity[] = {
>
> Is there a reason for the extra 't' in severity?

It's a typo.

>
> > +     { .id = TEP_LOG_NONE, .name = "none" },
> > +     { .id = TEP_LOG_CRITICAL, .name = "crit" },
> > +     { .id = TEP_LOG_ERROR, .name = "err" },
> > +     { .id = TEP_LOG_WARNING, .name = "warn" },
> > +     { .id = TEP_LOG_INFO, .name = "info" },
> > +     { .id = TEP_LOG_DEBUG, .name = "debug" },
> > +     { .id = TEP_LOG_ALL, .name = "all" },
> > +};
> > +
> > +int trace_set_verbose(char *level)
> > +{
> > +     int id;
> > +
> > +     if (!level)
> > +             return -1;
> > +
> > +     if (isdigit(level[0])) {
> > +             id = atoi(level);
> > +             if (id >= TEP_LOG_NONE && id <= TEP_LOG_ALL) {
> > +                     tracecmd_set_loglevel(id);
> > +                     return 0;
> > +             }
> > +     } else {
> > +             int size = sizeof(log_severtity) / sizeof(log_severtity[0]);
>
> This can be shortened to:
>
>                 int size = ARRAY_SIZE(log_severity);
>
> Or at least we need to copy that macro to something that trace-cmd core has
> access to (see lib/trace-cmd/include/private/trace-cmd-private.h)
>
>
> > +             int i;
> > +
> > +             for (i = 0; i < size; i++) {
> > +                     if (strlen(level) >= strlen(log_severtity[i].name) &&
>
> Why the strlen test? It's OK to pass in "debugignorme"? What's the use case
> for this?
>
> Anyway, I applied the first two patches of this series.
>
> -- Steve
>
>
> > +                         !strncmp(level, log_severtity[i].name, strlen(log_severtity[i].name))) {
> > +                             tracecmd_set_loglevel(log_severtity[i].id);
> > +                             return 0;
> > +                     }
> > +             }
> > +     }
> > +
> > +     return -1;
> > +}
> >
> >  /**
> >   * struct command
>
diff mbox series

Patch

diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h
index b3997d00..8da07c83 100644
--- a/tracecmd/include/trace-local.h
+++ b/tracecmd/include/trace-local.h
@@ -39,6 +39,8 @@  void usage(char **argv);
 extern int silence_warnings;
 extern int show_status;
 
+int trace_set_verbose(char *level);
+
 struct pid_record_data {
 	int			pid;
 	int			brass[2];
diff --git a/tracecmd/trace-cmd.c b/tracecmd/trace-cmd.c
index 60cd3ea1..4e4a73af 100644
--- a/tracecmd/trace-cmd.c
+++ b/tracecmd/trace-cmd.c
@@ -45,6 +45,47 @@  void *malloc_or_die(unsigned int size)
 	return data;
 }
 
+static struct trace_log_severity {
+	int		id;
+	const char	*name;
+} log_severtity[] = {
+	{ .id = TEP_LOG_NONE, .name = "none" },
+	{ .id = TEP_LOG_CRITICAL, .name = "crit" },
+	{ .id = TEP_LOG_ERROR, .name = "err" },
+	{ .id = TEP_LOG_WARNING, .name = "warn" },
+	{ .id = TEP_LOG_INFO, .name = "info" },
+	{ .id = TEP_LOG_DEBUG, .name = "debug" },
+	{ .id = TEP_LOG_ALL, .name = "all" },
+};
+
+int trace_set_verbose(char *level)
+{
+	int id;
+
+	if (!level)
+		return -1;
+
+	if (isdigit(level[0])) {
+		id = atoi(level);
+		if (id >= TEP_LOG_NONE && id <= TEP_LOG_ALL) {
+			tracecmd_set_loglevel(id);
+			return 0;
+		}
+	} else {
+		int size = sizeof(log_severtity) / sizeof(log_severtity[0]);
+		int i;
+
+		for (i = 0; i < size; i++) {
+			if (strlen(level) >= strlen(log_severtity[i].name) &&
+			    !strncmp(level, log_severtity[i].name, strlen(log_severtity[i].name))) {
+				tracecmd_set_loglevel(log_severtity[i].id);
+				return 0;
+			}
+		}
+	}
+
+	return -1;
+}
 
 /**
  * struct command