All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] log: allow for message continuation
@ 2020-09-17 12:19 Heinrich Schuchardt
  2020-09-17 12:19 ` [PATCH 1/2] " Heinrich Schuchardt
  2020-09-17 12:19 ` [PATCH 2/2] test: log: test " Heinrich Schuchardt
  0 siblings, 2 replies; 13+ messages in thread
From: Heinrich Schuchardt @ 2020-09-17 12:19 UTC (permalink / raw)
  To: u-boot

With the first patch it becomes possible to continue a log message with
the same log level and category as the previous messages.

We need this facility to convert pr_cont() to use our logging drivers.

The second patch provides a unit test.

This patch is a prerequisite:

[PATCH 1/1] log: mute messages generated by log drivers
https://lists.denx.de/pipermail/u-boot/2020-September/426418.html

Heinrich Schuchardt (2):
  log: allow for message continuation
  test: log: test message continuation

 common/log.c            | 23 ++++++++++++++----
 doc/develop/logging.rst |  6 +++++
 include/log.h           |  2 ++
 test/log/Makefile       |  4 +++-
 test/log/cont_test.c    | 52 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 81 insertions(+), 6 deletions(-)
 create mode 100644 test/log/cont_test.c

--
2.28.0

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/2] log: allow for message continuation
  2020-09-17 12:19 [PATCH 0/2] log: allow for message continuation Heinrich Schuchardt
@ 2020-09-17 12:19 ` Heinrich Schuchardt
  2020-09-22 18:48   ` Simon Glass
  2020-09-17 12:19 ` [PATCH 2/2] test: log: test " Heinrich Schuchardt
  1 sibling, 1 reply; 13+ messages in thread
From: Heinrich Schuchardt @ 2020-09-17 12:19 UTC (permalink / raw)
  To: u-boot

Some drivers use macro pr_cont() for continuing a message sent via printk.
Hence if we want to convert printk messaging to using the logging system,
we must support continuation of log messages too.

As pr_cont() does not provide a message level we need a means of
remembering the last log level.

With the patch a pseudo log level LOGL_CONT as well as a pseudo log
category LOGC_CONT are introduced. Using these results in the application
of the same log level and category as in the previous log message.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 common/log.c            | 23 ++++++++++++++++++-----
 doc/develop/logging.rst |  6 ++++++
 include/log.h           |  2 ++
 3 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/common/log.c b/common/log.c
index 9a5f100da3..bafc09f263 100644
--- a/common/log.c
+++ b/common/log.c
@@ -183,10 +183,12 @@ static bool log_passes_filters(struct log_device *ldev, struct log_rec *rec)
  * log_dispatch() - Send a log record to all log devices for processing
  *
  * The log record is sent to each log device in turn, skipping those which have
- * filters which block the record
+ * filters which block the record.
  *
- * @rec: Log record to dispatch
- * @return 0 (meaning success)
+ * All log messages created while processing log record @rec are ignored.
+ *
+ * @rec:	log record to dispatch
+ * Return:	0 msg sent, 1 msg not sent while already dispatching another msg
  */
 static int log_dispatch(struct log_rec *rec)
 {
@@ -199,7 +201,7 @@ static int log_dispatch(struct log_rec *rec)
 	 * as this might result in infinite recursion.
 	 */
 	if (processing_msg)
-		return 0;
+		return 1;

 	/* Emit message */
 	processing_msg = 1;
@@ -214,10 +216,18 @@ static int log_dispatch(struct log_rec *rec)
 int _log(enum log_category_t cat, enum log_level_t level, const char *file,
 	 int line, const char *func, const char *fmt, ...)
 {
+	static enum log_category_t logc_prev = LOGC_NONE;
+	static enum log_level_t logl_prev = LOGL_INFO;
 	char buf[CONFIG_SYS_CBSIZE];
 	struct log_rec rec;
 	va_list args;

+	/* Check for message continuation */
+	if (cat == LOGC_CONT)
+		cat = logc_prev;
+	if (level == LOGL_CONT)
+		level = logl_prev;
+
 	rec.cat = cat;
 	rec.level = level;
 	rec.file = file;
@@ -232,7 +242,10 @@ int _log(enum log_category_t cat, enum log_level_t level, const char *file,
 			gd->log_drop_count++;
 		return -ENOSYS;
 	}
-	log_dispatch(&rec);
+	if (!log_dispatch(&rec)) {
+		logc_prev = cat;
+		logl_prev = level;
+	}

 	return 0;
 }
diff --git a/doc/develop/logging.rst b/doc/develop/logging.rst
index 7ce8482ab6..c36f6bbbe4 100644
--- a/doc/develop/logging.rst
+++ b/doc/develop/logging.rst
@@ -38,6 +38,9 @@ There are a number logging levels available, in increasing order of verbosity:
 * LOGL_DEBUG_CONTENT - Debug message showing full message content
 * LOGL_DEBUG_IO - Debug message showing hardware I/O access

+To continue a log message in a separate call of function log() use
+
+* LOGL_CONT - Use same log level as in previous call

 Logging category
 ----------------
@@ -56,6 +59,9 @@ The following main categories are defined:
 * LOGC_DT - Related to device tree control
 * LOGC_EFI - Related to EFI implementation

+To continue a log message in a separate call of function log() use
+
+* LOGC_CONT - Use same category as in previous call

 Enabling logging
 ----------------
diff --git a/include/log.h b/include/log.h
index 2859ce1f2e..567cd32d34 100644
--- a/include/log.h
+++ b/include/log.h
@@ -35,6 +35,7 @@ enum log_level_t {

 	LOGL_FIRST = LOGL_EMERG,
 	LOGL_MAX = LOGL_DEBUG_IO,
+	LOGL_CONT = -1,		/* Use same log level as in previous call */
 };

 /**
@@ -60,6 +61,7 @@ enum log_category_t {

 	LOGC_COUNT,	/* Number of log categories */
 	LOGC_END,	/* Sentinel value for a list of log categories */
+	LOGC_CONT = -1,	/* Use same category as in previous call */
 };

 /* Helper to cast a uclass ID to a log category */
--
2.28.0

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/2] test: log: test message continuation
  2020-09-17 12:19 [PATCH 0/2] log: allow for message continuation Heinrich Schuchardt
  2020-09-17 12:19 ` [PATCH 1/2] " Heinrich Schuchardt
@ 2020-09-17 12:19 ` Heinrich Schuchardt
  2020-09-22 18:48   ` Simon Glass
  2020-10-10 21:28   ` Tom Rini
  1 sibling, 2 replies; 13+ messages in thread
From: Heinrich Schuchardt @ 2020-09-17 12:19 UTC (permalink / raw)
  To: u-boot

Provide a unit test checking that a continuation message will use the same
log level and log category as the previous message.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 test/log/Makefile    |  4 +++-
 test/log/cont_test.c | 52 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 1 deletion(-)
 create mode 100644 test/log/cont_test.c

diff --git a/test/log/Makefile b/test/log/Makefile
index 4c92550f6e..1f91c90422 100644
--- a/test/log/Makefile
+++ b/test/log/Makefile
@@ -12,7 +12,9 @@ ifdef CONFIG_SANDBOX
 obj-$(CONFIG_LOG_SYSLOG) += syslog_test.o
 endif

-ifndef CONFIG_LOG
+ifdef CONFIG_LOG
+obj-$(CONFIG_CONSOLE_RECORD) += cont_test.o
+else
 obj-$(CONFIG_CONSOLE_RECORD) += nolog_test.o
 endif

diff --git a/test/log/cont_test.c b/test/log/cont_test.c
new file mode 100644
index 0000000000..68ca1d262c
--- /dev/null
+++ b/test/log/cont_test.c
@@ -0,0 +1,52 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2020, Heinrich Schuchardt <xypron.glpk@gmx.de>
+ *
+ * Test continuation of log messages.
+ */
+
+#include <common.h>
+#include <console.h>
+#include <test/log.h>
+#include <test/test.h>
+#include <test/suites.h>
+#include <test/ut.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+#define BUFFSIZE 64
+
+static int log_test_cont(struct unit_test_state *uts)
+{
+	int log_fmt;
+	int log_level;
+
+	log_fmt = gd->log_fmt;
+	log_level = gd->default_log_level;
+
+	/* Write two messages, the second continuing the first */
+	gd->log_fmt = (1 << LOGF_CAT) | (1 << LOGF_LEVEL) | (1 << LOGF_MSG);
+	gd->default_log_level = LOGL_INFO;
+	console_record_reset_enable();
+	log(LOGC_ARCH, LOGL_ERR, "ea%d ", 1);
+	log(LOGC_CONT, LOGL_CONT, "cc%d\n", 2);
+	gd->default_log_level = log_level;
+	gd->log_fmt = log_fmt;
+	gd->flags &= ~GD_FLG_RECORD;
+	ut_assertok(ut_check_console_line(uts, "ERR.arch, ea1 ERR.arch, cc2"));
+	ut_assertok(ut_check_console_end(uts));
+
+	/* Write a third message which is not a continuation */
+	gd->log_fmt = (1 << LOGF_CAT) | (1 << LOGF_LEVEL) | (1 << LOGF_MSG);
+	gd->default_log_level = LOGL_INFO;
+	console_record_reset_enable();
+	log(LOGC_EFI, LOGL_INFO, "ie%d\n", 3);
+	gd->default_log_level = log_level;
+	gd->log_fmt = log_fmt;
+	gd->flags &= ~GD_FLG_RECORD;
+	ut_assertok(ut_check_console_line(uts, "INFO.efi, ie3"));
+	ut_assertok(ut_check_console_end(uts));
+
+	return 0;
+}
+LOG_TEST(log_test_cont);
--
2.28.0

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 1/2] log: allow for message continuation
  2020-09-17 12:19 ` [PATCH 1/2] " Heinrich Schuchardt
@ 2020-09-22 18:48   ` Simon Glass
  2020-09-22 19:10     ` Heinrich Schuchardt
  0 siblings, 1 reply; 13+ messages in thread
From: Simon Glass @ 2020-09-22 18:48 UTC (permalink / raw)
  To: u-boot

Hi Heinrich,

On Thu, 17 Sep 2020 at 06:19, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> Some drivers use macro pr_cont() for continuing a message sent via printk.
> Hence if we want to convert printk messaging to using the logging system,
> we must support continuation of log messages too.
>
> As pr_cont() does not provide a message level we need a means of
> remembering the last log level.
>
> With the patch a pseudo log level LOGL_CONT as well as a pseudo log
> category LOGC_CONT are introduced. Using these results in the application
> of the same log level and category as in the previous log message.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  common/log.c            | 23 ++++++++++++++++++-----
>  doc/develop/logging.rst |  6 ++++++
>  include/log.h           |  2 ++
>  3 files changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/common/log.c b/common/log.c
> index 9a5f100da3..bafc09f263 100644
> --- a/common/log.c
> +++ b/common/log.c
> @@ -183,10 +183,12 @@ static bool log_passes_filters(struct log_device *ldev, struct log_rec *rec)
>   * log_dispatch() - Send a log record to all log devices for processing
>   *
>   * The log record is sent to each log device in turn, skipping those which have
> - * filters which block the record
> + * filters which block the record.
>   *
> - * @rec: Log record to dispatch
> - * @return 0 (meaning success)
> + * All log messages created while processing log record @rec are ignored.
> + *
> + * @rec:       log record to dispatch
> + * Return:     0 msg sent, 1 msg not sent while already dispatching another msg
>   */
>  static int log_dispatch(struct log_rec *rec)
>  {
> @@ -199,7 +201,7 @@ static int log_dispatch(struct log_rec *rec)
>          * as this might result in infinite recursion.
>          */
>         if (processing_msg)
> -               return 0;
> +               return 1;
>
>         /* Emit message */
>         processing_msg = 1;
> @@ -214,10 +216,18 @@ static int log_dispatch(struct log_rec *rec)
>  int _log(enum log_category_t cat, enum log_level_t level, const char *file,
>          int line, const char *func, const char *fmt, ...)
>  {
> +       static enum log_category_t logc_prev = LOGC_NONE;
> +       static enum log_level_t logl_prev = LOGL_INFO;

I don't think we can use static variables in logging. Perhaps we can
use gobal_data?


>         char buf[CONFIG_SYS_CBSIZE];
>         struct log_rec rec;
>         va_list args;
>
> +       /* Check for message continuation */
> +       if (cat == LOGC_CONT)

Regards,
Simon

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 2/2] test: log: test message continuation
  2020-09-17 12:19 ` [PATCH 2/2] test: log: test " Heinrich Schuchardt
@ 2020-09-22 18:48   ` Simon Glass
  2020-10-10 21:28   ` Tom Rini
  1 sibling, 0 replies; 13+ messages in thread
From: Simon Glass @ 2020-09-22 18:48 UTC (permalink / raw)
  To: u-boot

On Thu, 17 Sep 2020 at 06:19, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> Provide a unit test checking that a continuation message will use the same
> log level and log category as the previous message.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  test/log/Makefile    |  4 +++-
>  test/log/cont_test.c | 52 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 55 insertions(+), 1 deletion(-)
>  create mode 100644 test/log/cont_test.c

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/2] log: allow for message continuation
  2020-09-22 18:48   ` Simon Glass
@ 2020-09-22 19:10     ` Heinrich Schuchardt
  2020-09-22 22:03       ` Simon Glass
  0 siblings, 1 reply; 13+ messages in thread
From: Heinrich Schuchardt @ 2020-09-22 19:10 UTC (permalink / raw)
  To: u-boot

On 9/22/20 8:48 PM, Simon Glass wrote:
> Hi Heinrich,
>
> On Thu, 17 Sep 2020 at 06:19, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>> Some drivers use macro pr_cont() for continuing a message sent via printk.
>> Hence if we want to convert printk messaging to using the logging system,
>> we must support continuation of log messages too.
>>
>> As pr_cont() does not provide a message level we need a means of
>> remembering the last log level.
>>
>> With the patch a pseudo log level LOGL_CONT as well as a pseudo log
>> category LOGC_CONT are introduced. Using these results in the application
>> of the same log level and category as in the previous log message.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>>  common/log.c            | 23 ++++++++++++++++++-----
>>  doc/develop/logging.rst |  6 ++++++
>>  include/log.h           |  2 ++
>>  3 files changed, 26 insertions(+), 5 deletions(-)
>>
>> diff --git a/common/log.c b/common/log.c
>> index 9a5f100da3..bafc09f263 100644
>> --- a/common/log.c
>> +++ b/common/log.c
>> @@ -183,10 +183,12 @@ static bool log_passes_filters(struct log_device *ldev, struct log_rec *rec)
>>   * log_dispatch() - Send a log record to all log devices for processing
>>   *
>>   * The log record is sent to each log device in turn, skipping those which have
>> - * filters which block the record
>> + * filters which block the record.
>>   *
>> - * @rec: Log record to dispatch
>> - * @return 0 (meaning success)
>> + * All log messages created while processing log record @rec are ignored.
>> + *
>> + * @rec:       log record to dispatch
>> + * Return:     0 msg sent, 1 msg not sent while already dispatching another msg
>>   */
>>  static int log_dispatch(struct log_rec *rec)
>>  {
>> @@ -199,7 +201,7 @@ static int log_dispatch(struct log_rec *rec)
>>          * as this might result in infinite recursion.
>>          */
>>         if (processing_msg)
>> -               return 0;
>> +               return 1;
>>
>>         /* Emit message */
>>         processing_msg = 1;
>> @@ -214,10 +216,18 @@ static int log_dispatch(struct log_rec *rec)
>>  int _log(enum log_category_t cat, enum log_level_t level, const char *file,
>>          int line, const char *func, const char *fmt, ...)
>>  {
>> +       static enum log_category_t logc_prev = LOGC_NONE;
>> +       static enum log_level_t logl_prev = LOGL_INFO;
>
> I don't think we can use static variables in logging. Perhaps we can
> use gobal_data?

Are you worried about relocation?

The initialization of the global data fields should be done in
log_init() before gd->flags |= GD_FLG_LOG_READY; I assume.

Is the rest ok for you?

Best regards

Heinrich

>
>
>>         char buf[CONFIG_SYS_CBSIZE];
>>         struct log_rec rec;
>>         va_list args;
>>
>> +       /* Check for message continuation */
>> +       if (cat == LOGC_CONT)
>
> Regards,
> Simon
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/2] log: allow for message continuation
  2020-09-22 19:10     ` Heinrich Schuchardt
@ 2020-09-22 22:03       ` Simon Glass
  2020-10-05  1:41         ` Simon Glass
  0 siblings, 1 reply; 13+ messages in thread
From: Simon Glass @ 2020-09-22 22:03 UTC (permalink / raw)
  To: u-boot

Hi Heinrich,

On Tue, 22 Sep 2020 at 13:10, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 9/22/20 8:48 PM, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Thu, 17 Sep 2020 at 06:19, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >>
> >> Some drivers use macro pr_cont() for continuing a message sent via printk.
> >> Hence if we want to convert printk messaging to using the logging system,
> >> we must support continuation of log messages too.
> >>
> >> As pr_cont() does not provide a message level we need a means of
> >> remembering the last log level.
> >>
> >> With the patch a pseudo log level LOGL_CONT as well as a pseudo log
> >> category LOGC_CONT are introduced. Using these results in the application
> >> of the same log level and category as in the previous log message.
> >>
> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >> ---
> >>  common/log.c            | 23 ++++++++++++++++++-----
> >>  doc/develop/logging.rst |  6 ++++++
> >>  include/log.h           |  2 ++
> >>  3 files changed, 26 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/common/log.c b/common/log.c
> >> index 9a5f100da3..bafc09f263 100644
> >> --- a/common/log.c
> >> +++ b/common/log.c
> >> @@ -183,10 +183,12 @@ static bool log_passes_filters(struct log_device *ldev, struct log_rec *rec)
> >>   * log_dispatch() - Send a log record to all log devices for processing
> >>   *
> >>   * The log record is sent to each log device in turn, skipping those which have
> >> - * filters which block the record
> >> + * filters which block the record.
> >>   *
> >> - * @rec: Log record to dispatch
> >> - * @return 0 (meaning success)
> >> + * All log messages created while processing log record @rec are ignored.
> >> + *
> >> + * @rec:       log record to dispatch
> >> + * Return:     0 msg sent, 1 msg not sent while already dispatching another msg
> >>   */
> >>  static int log_dispatch(struct log_rec *rec)
> >>  {
> >> @@ -199,7 +201,7 @@ static int log_dispatch(struct log_rec *rec)
> >>          * as this might result in infinite recursion.
> >>          */
> >>         if (processing_msg)
> >> -               return 0;
> >> +               return 1;
> >>
> >>         /* Emit message */
> >>         processing_msg = 1;
> >> @@ -214,10 +216,18 @@ static int log_dispatch(struct log_rec *rec)
> >>  int _log(enum log_category_t cat, enum log_level_t level, const char *file,
> >>          int line, const char *func, const char *fmt, ...)
> >>  {
> >> +       static enum log_category_t logc_prev = LOGC_NONE;
> >> +       static enum log_level_t logl_prev = LOGL_INFO;
> >
> > I don't think we can use static variables in logging. Perhaps we can
> > use gobal_data?
>
> Are you worried about relocation?

Yes, and SPL.

>
> The initialization of the global data fields should be done in
> log_init() before gd->flags |= GD_FLG_LOG_READY; I assume.

Yes.

>
> Is the rest ok for you?

Yes. If you are adding new things to global_data you could convert
default_log_level to a char to avoid using more space.


>
> Best regards
>
> Heinrich
>
> >
> >
> >>         char buf[CONFIG_SYS_CBSIZE];
> >>         struct log_rec rec;
> >>         va_list args;
> >>
> >> +       /* Check for message continuation */
> >> +       if (cat == LOGC_CONT)
> >
> > Regards,
> > Simon
> >
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/2] log: allow for message continuation
  2020-09-22 22:03       ` Simon Glass
@ 2020-10-05  1:41         ` Simon Glass
  0 siblings, 0 replies; 13+ messages in thread
From: Simon Glass @ 2020-10-05  1:41 UTC (permalink / raw)
  To: u-boot

Hi Heinrich,

On Tue, 22 Sep 2020 at 16:03, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Heinrich,
>
> On Tue, 22 Sep 2020 at 13:10, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >
> > On 9/22/20 8:48 PM, Simon Glass wrote:
> > > Hi Heinrich,
> > >
> > > On Thu, 17 Sep 2020 at 06:19, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> > >>
> > >> Some drivers use macro pr_cont() for continuing a message sent via printk.
> > >> Hence if we want to convert printk messaging to using the logging system,
> > >> we must support continuation of log messages too.
> > >>
> > >> As pr_cont() does not provide a message level we need a means of
> > >> remembering the last log level.
> > >>
> > >> With the patch a pseudo log level LOGL_CONT as well as a pseudo log
> > >> category LOGC_CONT are introduced. Using these results in the application
> > >> of the same log level and category as in the previous log message.
> > >>
> > >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > >> ---
> > >>  common/log.c            | 23 ++++++++++++++++++-----
> > >>  doc/develop/logging.rst |  6 ++++++
> > >>  include/log.h           |  2 ++
> > >>  3 files changed, 26 insertions(+), 5 deletions(-)
> > >>
> > >> diff --git a/common/log.c b/common/log.c
> > >> index 9a5f100da3..bafc09f263 100644
> > >> --- a/common/log.c
> > >> +++ b/common/log.c
> > >> @@ -183,10 +183,12 @@ static bool log_passes_filters(struct log_device *ldev, struct log_rec *rec)
> > >>   * log_dispatch() - Send a log record to all log devices for processing
> > >>   *
> > >>   * The log record is sent to each log device in turn, skipping those which have
> > >> - * filters which block the record
> > >> + * filters which block the record.
> > >>   *
> > >> - * @rec: Log record to dispatch
> > >> - * @return 0 (meaning success)
> > >> + * All log messages created while processing log record @rec are ignored.
> > >> + *
> > >> + * @rec:       log record to dispatch
> > >> + * Return:     0 msg sent, 1 msg not sent while already dispatching another msg
> > >>   */
> > >>  static int log_dispatch(struct log_rec *rec)
> > >>  {
> > >> @@ -199,7 +201,7 @@ static int log_dispatch(struct log_rec *rec)
> > >>          * as this might result in infinite recursion.
> > >>          */
> > >>         if (processing_msg)
> > >> -               return 0;
> > >> +               return 1;
> > >>
> > >>         /* Emit message */
> > >>         processing_msg = 1;
> > >> @@ -214,10 +216,18 @@ static int log_dispatch(struct log_rec *rec)
> > >>  int _log(enum log_category_t cat, enum log_level_t level, const char *file,
> > >>          int line, const char *func, const char *fmt, ...)
> > >>  {
> > >> +       static enum log_category_t logc_prev = LOGC_NONE;
> > >> +       static enum log_level_t logl_prev = LOGL_INFO;
> > >
> > > I don't think we can use static variables in logging. Perhaps we can
> > > use gobal_data?
> >
> > Are you worried about relocation?
>
> Yes, and SPL.
>
> >
> > The initialization of the global data fields should be done in
> > log_init() before gd->flags |= GD_FLG_LOG_READY; I assume.
>
> Yes.
>
> >
> > Is the rest ok for you?
>
> Yes. If you are adding new things to global_data you could convert
> default_log_level to a char to avoid using more space.
>

Also I notice that the processing_msg variable in log.c crashes
logging in SPL/TPL. Can you please move this to global_data too?

Regards,
Simon

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 2/2] test: log: test message continuation
  2020-09-17 12:19 ` [PATCH 2/2] test: log: test " Heinrich Schuchardt
  2020-09-22 18:48   ` Simon Glass
@ 2020-10-10 21:28   ` Tom Rini
  2020-10-10 21:47     ` Sean Anderson
  2020-10-11  8:11     ` Heinrich Schuchardt
  1 sibling, 2 replies; 13+ messages in thread
From: Tom Rini @ 2020-10-10 21:28 UTC (permalink / raw)
  To: u-boot

On Thu, Sep 17, 2020 at 02:19:02PM +0200, Heinrich Schuchardt wrote:

> Provide a unit test checking that a continuation message will use the same
> log level and log category as the previous message.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Reviewed-by: Simon Glass <sjg@chromium.org>
[snip]
> +	log(LOGC_CONT, LOGL_CONT, "cc%d\n", 2);

These new values aren't defined, or I missed some other series.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201010/4fa914cd/attachment.sig>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 2/2] test: log: test message continuation
  2020-10-10 21:28   ` Tom Rini
@ 2020-10-10 21:47     ` Sean Anderson
  2020-10-10 21:51       ` Tom Rini
  2020-10-11  8:11     ` Heinrich Schuchardt
  1 sibling, 1 reply; 13+ messages in thread
From: Sean Anderson @ 2020-10-10 21:47 UTC (permalink / raw)
  To: u-boot

On 10/10/20 5:28 PM, Tom Rini wrote:
> On Thu, Sep 17, 2020 at 02:19:02PM +0200, Heinrich Schuchardt wrote:
> 
>> Provide a unit test checking that a continuation message will use the same
>> log level and log category as the previous message.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
> [snip]
>> +	log(LOGC_CONT, LOGL_CONT, "cc%d\n", 2);
> 
> These new values aren't defined, or I missed some other series.  Thanks!
> 

They should be defined in the first patch

On 9/17/20 8:19 AM, Heinrich Schuchardt wrote:
> diff --git a/include/log.h b/include/log.h
> index 2859ce1f2e..567cd32d34 100644
> --- a/include/log.h
> +++ b/include/log.h
> @@ -35,6 +35,7 @@ enum log_level_t {
> 
>  	LOGL_FIRST = LOGL_EMERG,
>  	LOGL_MAX = LOGL_DEBUG_IO,
> +	LOGL_CONT = -1,		/* Use same log level as in previous call */
>  };
> 
>  /**
> @@ -60,6 +61,7 @@ enum log_category_t {
> 
>  	LOGC_COUNT,	/* Number of log categories */
>  	LOGC_END,	/* Sentinel value for a list of log categories */
> +	LOGC_CONT = -1,	/* Use same category as in previous call */
>  };
> 
>  /* Helper to cast a uclass ID to a log category */
> --
> 2.28.0
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 2/2] test: log: test message continuation
  2020-10-10 21:47     ` Sean Anderson
@ 2020-10-10 21:51       ` Tom Rini
  2020-10-11  8:09         ` Heinrich Schuchardt
  0 siblings, 1 reply; 13+ messages in thread
From: Tom Rini @ 2020-10-10 21:51 UTC (permalink / raw)
  To: u-boot

On Sat, Oct 10, 2020 at 05:47:26PM -0400, Sean Anderson wrote:
> On 10/10/20 5:28 PM, Tom Rini wrote:
> > On Thu, Sep 17, 2020 at 02:19:02PM +0200, Heinrich Schuchardt wrote:
> > 
> >> Provide a unit test checking that a continuation message will use the same
> >> log level and log category as the previous message.
> >>
> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >> Reviewed-by: Simon Glass <sjg@chromium.org>
> > [snip]
> >> +	log(LOGC_CONT, LOGL_CONT, "cc%d\n", 2);
> > 
> > These new values aren't defined, or I missed some other series.  Thanks!
> > 
> 
> They should be defined in the first patch

and I see 1/2 has changes requested, so I shoulda deferred this one.
Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201010/d41b36cb/attachment.sig>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 2/2] test: log: test message continuation
  2020-10-10 21:51       ` Tom Rini
@ 2020-10-11  8:09         ` Heinrich Schuchardt
  0 siblings, 0 replies; 13+ messages in thread
From: Heinrich Schuchardt @ 2020-10-11  8:09 UTC (permalink / raw)
  To: u-boot

On 10/10/20 11:51 PM, Tom Rini wrote:
> On Sat, Oct 10, 2020 at 05:47:26PM -0400, Sean Anderson wrote:
>> On 10/10/20 5:28 PM, Tom Rini wrote:
>>> On Thu, Sep 17, 2020 at 02:19:02PM +0200, Heinrich Schuchardt wrote:
>>>
>>>> Provide a unit test checking that a continuation message will use the same
>>>> log level and log category as the previous message.
>>>>
>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>> [snip]
>>>> +	log(LOGC_CONT, LOGL_CONT, "cc%d\n", 2);
>>>
>>> These new values aren't defined, or I missed some other series.  Thanks!
>>>
>>
>> They should be defined in the first patch
>
> and I see 1/2 has changes requested, so I shoulda deferred this one.
> Thanks!
>
Hello Tom,

I am still waiting on the "doc: global data pointer" series to be merged
as this will be the basis for adjusting the log continuation series.

https://patchwork.ozlabs.org/project/uboot/list/?series=205937

But I will be patient as it seems that you have been drowned in patches
assigned to you.

Best regards

Heinrich

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 2/2] test: log: test message continuation
  2020-10-10 21:28   ` Tom Rini
  2020-10-10 21:47     ` Sean Anderson
@ 2020-10-11  8:11     ` Heinrich Schuchardt
  1 sibling, 0 replies; 13+ messages in thread
From: Heinrich Schuchardt @ 2020-10-11  8:11 UTC (permalink / raw)
  To: u-boot

On 10/10/20 11:28 PM, Tom Rini wrote:
> On Thu, Sep 17, 2020 at 02:19:02PM +0200, Heinrich Schuchardt wrote:
>
>> Provide a unit test checking that a continuation message will use the same
>> log level and log category as the previous message.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
> [snip]
>> +	log(LOGC_CONT, LOGL_CONT, "cc%d\n", 2);
>
> These new values aren't defined, or I missed some other series.  Thanks!
>

See here:

https://patchwork.ozlabs.org/project/uboot/patch/20200917121902.57403-2-xypron.glpk at gmx.de/

But the series has to be reworked.

Best regards

Heinrich

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-10-11  8:11 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-17 12:19 [PATCH 0/2] log: allow for message continuation Heinrich Schuchardt
2020-09-17 12:19 ` [PATCH 1/2] " Heinrich Schuchardt
2020-09-22 18:48   ` Simon Glass
2020-09-22 19:10     ` Heinrich Schuchardt
2020-09-22 22:03       ` Simon Glass
2020-10-05  1:41         ` Simon Glass
2020-09-17 12:19 ` [PATCH 2/2] test: log: test " Heinrich Schuchardt
2020-09-22 18:48   ` Simon Glass
2020-10-10 21:28   ` Tom Rini
2020-10-10 21:47     ` Sean Anderson
2020-10-10 21:51       ` Tom Rini
2020-10-11  8:09         ` Heinrich Schuchardt
2020-10-11  8:11     ` Heinrich Schuchardt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.