logbuff: Prevent an infinite loop for console output

When using 'logbuff' as stdout and the console loglevel is greater than a message's loglevel it is supposed to be both logged, and printed to the console. The logbuff_printk() function is responsible for both logging and displaying the message. However, logbuff_printk() previously used printf() to print the message to the console. The printf() call would eventually end up back in logbuff_printk(), and an infinite loop would occur which would hang a board.

Using serial_puts() instead of printf() in logbuff_printk() avoids the recursion and resolves the issue.

---
 common/cmd_log.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

index e3ebe96..baffeb1 100644
@@ -313,9 +313,10 @@ static int logbuff_printk(const char *line)
 				break;
 			}
 		}
-		if (msg_level < console_loglevel) {
-			printf("%s", msg);
-		}
+
+		if (msg_level < console_loglevel)
+			serial_puts(msg);
+
 		if (line_feed)
 			msg_level = -1;
 	}

1.6.2.1

Hi Peter,

Hm. What if a board has "stdout" set to "lcd" or "nc" or any other device? Do we really want the text to be output on the serial console then? Doesn't this break the whole "stdout" concept?

Cheers
Detlev

Some people unfortunately like  jumping up and down about spaces but not code.
[...]   I'd rather read good poetry written in very bad hand writing than bad
poetry written in beautiful handwriting, and I think the same is true of code.
           -- Alan Cox <[email protected]>
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

Yes, it does break the stdout concept, but I went with the current patch for 3 reasons:
1. Most anything is better than the current board lockup that occurs without the change.

2. For most of the boot process serial_puts() is used to output messages (ie prior to GD_FLG_DEVINIT is set in gd->flags). Continuing to use the same output method after GD_FLG_DEVINIT is set makes some degree of sense.

3. A proper fix would be more involved. For example, perhaps allowing the stdout variable to support multiple devices (eg "stdout=logbuff,lcd") would be a more elegant solution. In any case, to use both the logbuff and non-serial port for output would require much more extensive changes than this 1-line change. I don't use the logbuff, so am not so interested in spending the time to fix it. The fact that no one else ran into this bug implies no on else is using it either.

I agree this fix isn't the best, but its better than the bug in my opinion. Ideally someone who uses the logbuff could provide a more elegant fix. Any takers?

Best,
Peter

Dear Peter Tyser,

OK, then let's try and come up with a solution that does NOT break this.

Even if I was willing to buy this one...

... this does not make sense to me. As Detlev pointed out, it breaks the concept of I/O redirection through stdout.

I don't want to replace one bug (that bites you) with another one ( that bites somebody else).

Best regards,

Wolfgang Denk

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I thought my people would grow tired of killing. But you were  right,
they  see it is easier than trading. And it has its pleasures. I feel
it myself. Like the hunt, but with richer rewards.
	-- Apella, "A Private Little War", stardate 4211.8

Hi Wolfgang,

I think the bug that I'm introducing (continuing to use serial output/ignoring the value of stdout) is much better than a hard lockup (the current bug), but I definitely see your point. I'm not a user of the logbuff or LCDs, etc at this point in time, so feel free to do what you'd like with the patch.

Best,
Peter