[RFC] env: Fix errors printing on env loading

Message ID 20180718213736.22650-1-semen.protsenko@linaro.org
State New
Headers show
Series
  • [RFC] env: Fix errors printing on env loading
Related show

Commit Message

Sam Protsenko July 18, 2018, 9:37 p.m.
This is just a draft to discuss ideas related to "Make U-Boot log great
again" thread.

With this patch we will have something like this:

    Attempting to load environment from FAT:
    MMC: no card present
    ** Bad device mmc 0 **
    Failed (-5)
    Attempting to load environment from MMC: OK

instead of this:

    Loading Environment from FAT... MMC: no card present
    ** Bad device mmc 0 **
    Failed (-5)
    Loading Environment from MMC... OK

The only way I see to do so is to use ASCII escape codes for moving the
cursor (in non-error case).

I'd also like to add prefixes to error messages, like it's done in [2],
but it requires adding one pointer to global data struct.

[1] https://en.wikipedia.org/wiki/ANSI_escape_code#CSI_sequences
[2] https://lists.denx.de/pipermail/u-boot/2018-July/335072.html

Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
---
 env/env.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

Comments

Wolfgang Denk July 19, 2018, 7:43 p.m. | #1
Dear Sam,

In message <20180718213736.22650-1-semen.protsenko@linaro.org> you wrote:
> This is just a draft to discuss ideas related to "Make U-Boot log great
> again" thread.
>
> With this patch we will have something like this:
>
>     Attempting to load environment from FAT:
>     MMC: no card present
>     ** Bad device mmc 0 **
>     Failed (-5)
>     Attempting to load environment from MMC: OK
>
> instead of this:
>
>     Loading Environment from FAT... MMC: no card present
>     ** Bad device mmc 0 **
>     Failed (-5)
>     Loading Environment from MMC... OK

The new output is worse, as it consumes even more lines of output.

> The only way I see to do so is to use ASCII escape codes for moving the
> cursor (in non-error case).

NAK!!!  Please keep in mind that output usually goes to a serial
port, and should not assume any speific "terminal" capabilities.
Even simple in-line "editing" like "overprinting by outputting
backspace characters causes a LOT of hassle when you try to parse
output for example in automatic test suites.

Adding more complex terminal control or other fancy stuff (like
colors etc) is a _strict_ no, no!

> I'd also like to add prefixes to error messages, like it's done in [2],
> but it requires adding one pointer to global data struct.

As I explained befoire, this has a lot of negative consequences.
You think about beauty, but please keep functionality and efficiency
(boot time) in mind!

Best regards,

Wolfgang Denk

Patch

diff --git a/env/env.c b/env/env.c
index 5c0842ac07..a674ac2eab 100644
--- a/env/env.c
+++ b/env/env.c
@@ -187,6 +187,7 @@  int env_load(void)
 
 	for (prio = 0; (drv = env_driver_lookup(ENVOP_LOAD, prio)); prio++) {
 		int ret;
+		char msg[75];
 
 		if (!drv->load)
 			continue;
@@ -194,12 +195,17 @@  int env_load(void)
 		if (!env_has_inited(drv->location))
 			continue;
 
-		printf("Loading Environment from %s... ", drv->name);
+		snprintf(msg, 75, "Attempting to load environment from %s:\n",
+			 drv->name);
+		puts(msg);
 		ret = drv->load();
-		if (ret)
+		if (ret) {
 			printf("Failed (%d)\n", ret);
-		else
-			printf("OK\n");
+		} else {
+			size_t len = strlen(msg);
+
+			printf("\033[1A\033[%zuC OK\n", len - 1);
+		}
 
 		if (!ret)
 			return 0;