diff mbox series

[v3,1/2] log: don't show function by default

Message ID 20200615192354.3675-2-xypron.glpk@gmx.de
State Superseded
Headers show
Series log: don't show function by default | expand

Commit Message

Heinrich Schuchardt June 15, 2020, 7:23 p.m. UTC 
The name of the function emitting a log message may be of interest for a
developer but is distracting for normal users. See the example below:

    try_load_entry() Booting: Debian

Make the default format for log messages customizable. By default show
only the message text.

Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
---
v3:
	replace #ifdef by IS_ENABLED()
---
 cmd/log.c                 |  4 ++--
 common/Kconfig            | 18 ++++++++++++++++++
 common/log.c              |  2 +-
 include/log.h             | 19 +++++++++++++++++--
 test/log/syslog_test.c    | 20 ++++++++++++++------
 test/py/tests/test_log.py |  2 ++
 6 files changed, 54 insertions(+), 11 deletions(-)

--
2.27.0

Comments

Simon Glass June 16, 2020, 11:31 p.m. UTC | #1 
Hi Heinrich,

On Mon, 15 Jun 2020 at 13:24, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> The name of the function emitting a log message may be of interest for a
> developer but is distracting for normal users. See the example below:
>
>     try_load_entry() Booting: Debian
>
> Make the default format for log messages customizable. By default show
> only the message text.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> ---
> v3:
>         replace #ifdef by IS_ENABLED()
> ---
>  cmd/log.c                 |  4 ++--
>  common/Kconfig            | 18 ++++++++++++++++++
>  common/log.c              |  2 +-
>  include/log.h             | 19 +++++++++++++++++--
>  test/log/syslog_test.c    | 20 ++++++++++++++------
>  test/py/tests/test_log.py |  2 ++
>  6 files changed, 54 insertions(+), 11 deletions(-)
>

Reviewed-by: Simon Glass <sjg at chromium.org>

nit below

[..]

> diff --git a/include/log.h b/include/log.h
> index df65398c04..21f9b75278 100644
> --- a/include/log.h
> +++ b/include/log.h
> @@ -12,6 +12,7 @@
>  #include <stdio.h>
>  #include <linker_lists.h>
>  #include <dm/uclass-id.h>
> +#include <linux/bitops.h>
>  #include <linux/list.h>
>
>  struct cmd_tbl;
> @@ -409,9 +410,7 @@ enum log_fmt {
>         LOGF_LINE,
>         LOGF_FUNC,
>         LOGF_MSG,
> -

Can we keep that? I makes it easier to see the 'real' enum values and
the ones that are used for maintenance below:

>         LOGF_COUNT,
> -       LOGF_DEFAULT = (1 << LOGF_FUNC) | (1 << LOGF_MSG),
>         LOGF_ALL = 0x3f,
>  };
>
> @@ -460,4 +459,20 @@ static inline int log_init(void)
>  }
>  #endif
>

Regards,
Simon
diff mbox series

Patch

diff --git a/cmd/log.c b/cmd/log.c
index 78352b2cb9..6afe6ead25 100644
--- a/cmd/log.c
+++ b/cmd/log.c
@@ -39,7 +39,7 @@  static int do_log_format(struct cmd_tbl *cmdtp, int flag, int argc,
 		const char *str = argv[1];

 		if (!strcmp(str, "default")) {
-			gd->log_fmt = LOGF_DEFAULT;
+			gd->log_fmt = log_get_default_format();
 		} else if (!strcmp(str, "all")) {
 			gd->log_fmt = LOGF_ALL;
 		} else {
@@ -139,7 +139,7 @@  static char log_help_text[] =
 	"log format <fmt> - set log output format. <fmt> is a string where\n"
 	"\teach letter indicates something that should be displayed:\n"
 	"\tc=category, l=level, F=file, L=line number, f=function, m=msg\n"
-	"\tor 'default', equivalent to 'fm', or 'all' for all\n"
+	"\tor 'default', or 'all' for all\n"
 	"log rec <category> <level> <file> <line> <func> <message> - "
 		"output a log record"
 	;
diff --git a/common/Kconfig b/common/Kconfig
index 7872bc46cd..60cae77f20 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -792,6 +792,24 @@  config TPL_LOG_CONSOLE

 endif

+config LOGF_FILE
+	bool "Show source file name in log messages by default"
+	help
+	  Show the source file name in log messages by default. This value
+	  can be overridden using the 'log format' command.
+
+config LOGF_LINE
+	bool "Show source line number in log messages by default"
+	help
+	  Show the source line number in log messages by default. This value
+	  can be overridden using the 'log format' command.
+
+config LOGF_FUNC
+	bool "Show function name in log messages by default"
+	help
+	  Show the function name in log messages by default. This value can
+	  be overridden using the 'log format' command.
+
 config LOG_ERROR_RETURN
 	bool "Log all functions which return an error"
 	help
diff --git a/common/log.c b/common/log.c
index c5b9b489ca..9005b3afb2 100644
--- a/common/log.c
+++ b/common/log.c
@@ -317,7 +317,7 @@  int log_init(void)
 	gd->flags |= GD_FLG_LOG_READY;
 	if (!gd->default_log_level)
 		gd->default_log_level = CONFIG_LOG_DEFAULT_LEVEL;
-	gd->log_fmt = LOGF_DEFAULT;
+	gd->log_fmt = log_get_default_format();

 	return 0;
 }
diff --git a/include/log.h b/include/log.h
index df65398c04..21f9b75278 100644
--- a/include/log.h
+++ b/include/log.h
@@ -12,6 +12,7 @@ 
 #include <stdio.h>
 #include <linker_lists.h>
 #include <dm/uclass-id.h>
+#include <linux/bitops.h>
 #include <linux/list.h>

 struct cmd_tbl;
@@ -409,9 +410,7 @@  enum log_fmt {
 	LOGF_LINE,
 	LOGF_FUNC,
 	LOGF_MSG,
-
 	LOGF_COUNT,
-	LOGF_DEFAULT = (1 << LOGF_FUNC) | (1 << LOGF_MSG),
 	LOGF_ALL = 0x3f,
 };

@@ -460,4 +459,20 @@  static inline int log_init(void)
 }
 #endif

+/**
+ * log_get_default_format() - get default log format
+ *
+ * The default log format is configurable via
+ * CONFIG_LOGF_FILE, CONFIG_LOGF_LINE, CONFIG_LOGF_FUNC.
+ *
+ * Return:	default log format
+ */
+static inline int log_get_default_format(void)
+{
+	return BIT(LOGF_MSG) |
+	       (IS_ENABLED(CONFIG_LOGF_FILE) ? BIT(LOGF_FILE) : 0) |
+	       (IS_ENABLED(CONFIG_LOGF_LINE) ? BIT(LOGF_LINE) : 0) |
+	       (IS_ENABLED(CONFIG_LOGF_FUNC) ? BIT(LOGF_FUNC) : 0);
+}
+
 #endif
diff --git a/test/log/syslog_test.c b/test/log/syslog_test.c
index 26536ebca7..120a8b2537 100644
--- a/test/log/syslog_test.c
+++ b/test/log/syslog_test.c
@@ -21,6 +21,8 @@ 

 DECLARE_GLOBAL_DATA_PTR;

+#define LOGF_TEST (BIT(LOGF_FUNC) | BIT(LOGF_MSG))
+
 /**
  * struct sb_log_env - private data for sandbox ethernet driver
  *
@@ -102,7 +104,7 @@  static int log_test_syslog_err(struct unit_test_state *uts)
 	int old_log_level = gd->default_log_level;
 	struct sb_log_env env;

-	gd->log_fmt = LOGF_DEFAULT;
+	gd->log_fmt = LOGF_TEST;
 	gd->default_log_level = LOGL_INFO;
 	env_set("ethact", "eth at 10002000");
 	env_set("log_hostname", "sandbox");
@@ -116,6 +118,7 @@  static int log_test_syslog_err(struct unit_test_state *uts)
 	/* Check that the callback function was called */
 	sandbox_eth_set_tx_handler(0, NULL);
 	gd->default_log_level = old_log_level;
+	gd->log_fmt = log_get_default_format();

 	return 0;
 }
@@ -132,7 +135,7 @@  static int log_test_syslog_warning(struct unit_test_state *uts)
 	int old_log_level = gd->default_log_level;
 	struct sb_log_env env;

-	gd->log_fmt = LOGF_DEFAULT;
+	gd->log_fmt = LOGF_TEST;
 	gd->default_log_level = LOGL_INFO;
 	env_set("ethact", "eth at 10002000");
 	env_set("log_hostname", "sandbox");
@@ -147,6 +150,7 @@  static int log_test_syslog_warning(struct unit_test_state *uts)
 	/* Check that the callback function was called */
 	ut_assertnull(env.expected);
 	gd->default_log_level = old_log_level;
+	gd->log_fmt = log_get_default_format();

 	return 0;
 }
@@ -163,7 +167,7 @@  static int log_test_syslog_notice(struct unit_test_state *uts)
 	int old_log_level = gd->default_log_level;
 	struct sb_log_env env;

-	gd->log_fmt = LOGF_DEFAULT;
+	gd->log_fmt = LOGF_TEST;
 	gd->default_log_level = LOGL_INFO;
 	env_set("ethact", "eth at 10002000");
 	env_set("log_hostname", "sandbox");
@@ -178,6 +182,7 @@  static int log_test_syslog_notice(struct unit_test_state *uts)
 	/* Check that the callback function was called */
 	ut_assertnull(env.expected);
 	gd->default_log_level = old_log_level;
+	gd->log_fmt = log_get_default_format();

 	return 0;
 }
@@ -194,7 +199,7 @@  static int log_test_syslog_info(struct unit_test_state *uts)
 	int old_log_level = gd->default_log_level;
 	struct sb_log_env env;

-	gd->log_fmt = LOGF_DEFAULT;
+	gd->log_fmt = LOGF_TEST;
 	gd->default_log_level = LOGL_INFO;
 	env_set("ethact", "eth at 10002000");
 	env_set("log_hostname", "sandbox");
@@ -209,6 +214,7 @@  static int log_test_syslog_info(struct unit_test_state *uts)
 	/* Check that the callback function was called */
 	ut_assertnull(env.expected);
 	gd->default_log_level = old_log_level;
+	gd->log_fmt = log_get_default_format();

 	return 0;
 }
@@ -225,7 +231,7 @@  static int log_test_syslog_debug(struct unit_test_state *uts)
 	int old_log_level = gd->default_log_level;
 	struct sb_log_env env;

-	gd->log_fmt = LOGF_DEFAULT;
+	gd->log_fmt = LOGF_TEST;
 	gd->default_log_level = LOGL_DEBUG;
 	env_set("ethact", "eth at 10002000");
 	env_set("log_hostname", "sandbox");
@@ -240,6 +246,7 @@  static int log_test_syslog_debug(struct unit_test_state *uts)
 	/* Check that the callback function was called */
 	ut_assertnull(env.expected);
 	gd->default_log_level = old_log_level;
+	gd->log_fmt = log_get_default_format();

 	return 0;
 }
@@ -259,7 +266,7 @@  static int log_test_syslog_nodebug(struct unit_test_state *uts)
 	int old_log_level = gd->default_log_level;
 	struct sb_log_env env;

-	gd->log_fmt = LOGF_DEFAULT;
+	gd->log_fmt = LOGF_TEST;
 	gd->default_log_level = LOGL_INFO;
 	env_set("ethact", "eth at 10002000");
 	env_set("log_hostname", "sandbox");
@@ -274,6 +281,7 @@  static int log_test_syslog_nodebug(struct unit_test_state *uts)
 	/* Check that the callback function was not called */
 	ut_assertnonnull(env.expected);
 	gd->default_log_level = old_log_level;
+	gd->log_fmt = log_get_default_format();

 	return 0;
 }
diff --git a/test/py/tests/test_log.py b/test/py/tests/test_log.py
index 75325fad61..ddc28f19ee 100644
--- a/test/py/tests/test_log.py
+++ b/test/py/tests/test_log.py
@@ -39,6 +39,8 @@  def test_log(u_boot_console):
         Returns:
             iterator containing the lines output from the command
         """
+        output = u_boot_console.run_command('log format fm')
+        assert output == ''
         with cons.log.section('basic'):
            output = u_boot_console.run_command('log test %d' % testnum)
         split = output.replace('\r', '').splitlines()