[U-Boot] [PATCH 02/31] environment, netconsole: reducing packages when printing environment variables

Actual, do_printenv () uses putc () to print the Environment- variables. If using netconsole, this will result in a lot of packages with single characters in it. With the new config option CONFIG_ENV_BUFFER_PRINT do_printenv () uses now puts () to print a complete Environmentvariable, so the number of packages when using netconsole are reduced.
Signed-off-by: Heiko Schocher hs@denx.de --- common/cmd_nvedit.c | 8 ++++++++ include/configs/mgcoge.h | 1 + include/configs/mgsuvd.h | 1 + 3 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c index 85025da..8f6310a 100644 --- a/common/cmd_nvedit.c +++ b/common/cmd_nvedit.c @@ -89,8 +89,12 @@ int do_printenv (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) for (i=0; env_get_char(i) != '\0'; i=nxt+1) { for (nxt=i; env_get_char(nxt) != '\0'; ++nxt) ; +#if defined(CONFIG_ENV_BUFFER_PRINT) + puts ((char *)( env_get_addr(nxt) + 1)); +#else for (k=i; k<nxt; ++k) putc(env_get_char(k)); +#endif putc ('\n');
if (ctrlc()) { @@ -118,11 +122,15 @@ int do_printenv (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) if (k < 0) { continue; } +#if defined(CONFIG_ENV_BUFFER_PRINT) + printf ("%s=%s\n", name, (char *)( env_get_addr(k))); +#else puts (name); putc ('='); while (k < nxt) putc(env_get_char(k++)); putc ('\n'); +#endif break; } if (k < 0) { diff --git a/include/configs/mgcoge.h b/include/configs/mgcoge.h index 233bee0..9416a03 100644 --- a/include/configs/mgcoge.h +++ b/include/configs/mgcoge.h @@ -139,6 +139,7 @@ #define CONFIG_ENV_OFFSET_REDUND (CONFIG_ENV_OFFSET + CONFIG_ENV_SECT_SIZE) #define CONFIG_ENV_SIZE_REDUND (CONFIG_ENV_SIZE) #endif /* CONFIG_ENV_IS_IN_FLASH */ +#define CONFIG_ENV_BUFFER_PRINT 1
/* enable I2C and select the hardware/software driver */ #undef CONFIG_HARD_I2C /* I2C with hardware support */ diff --git a/include/configs/mgsuvd.h b/include/configs/mgsuvd.h index 8f82751..698f0e7 100644 --- a/include/configs/mgsuvd.h +++ b/include/configs/mgsuvd.h @@ -143,6 +143,7 @@ /* Address and size of Redundant Environment Sector */ #define CONFIG_ENV_OFFSET_REDUND (CONFIG_ENV_OFFSET+CONFIG_ENV_SECT_SIZE) #define CONFIG_ENV_SIZE_REDUND (CONFIG_ENV_SIZE) +#define CONFIG_ENV_BUFFER_PRINT 1
/*----------------------------------------------------------------------- * Cache Configuration

Hi Heiko,
On Wednesday 28 January 2009, Heiko Schocher wrote:
Actual, do_printenv () uses putc () to print the Environment- variables. If using netconsole, this will result in a lot of packages with single characters in it. With the new config option CONFIG_ENV_BUFFER_PRINT do_printenv () uses now puts () to print a complete Environmentvariable, so the number of packages when using netconsole are reduced.
Wouldn't it make sense to use this new code (in the CONFIG_ENV_BUFFER_PRINT sections) unconditionally instead of the old version? The source would look nicer and the resulting image probably smaller.
Some nitpicking comments below.
Signed-off-by: Heiko Schocher hs@denx.de
common/cmd_nvedit.c | 8 ++++++++ include/configs/mgcoge.h | 1 + include/configs/mgsuvd.h | 1 + 3 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c index 85025da..8f6310a 100644 --- a/common/cmd_nvedit.c +++ b/common/cmd_nvedit.c @@ -89,8 +89,12 @@ int do_printenv (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) for (i=0; env_get_char(i) != '\0'; i=nxt+1) { for (nxt=i; env_get_char(nxt) != '\0'; ++nxt) ; +#if defined(CONFIG_ENV_BUFFER_PRINT)
puts ((char *)( env_get_addr(nxt) + 1));
Please stick with one coding-style rule, either "func ()" or "func()". Since this file already uses the "func()" style I suggest to use this version.
+#else for (k=i; k<nxt; ++k) putc(env_get_char(k)); +#endif putc ('\n');
Why not fold this "putc()" into the puts() above (replace it with printf)?
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================

Hello Stefan,
Stefan Roese wrote:
On Wednesday 28 January 2009, Heiko Schocher wrote:
Actual, do_printenv () uses putc () to print the Environment- variables. If using netconsole, this will result in a lot of packages with single characters in it. With the new config option CONFIG_ENV_BUFFER_PRINT do_printenv () uses now puts () to print a complete Environmentvariable, so the number of packages when using netconsole are reduced.
Wouldn't it make sense to use this new code (in the CONFIG_ENV_BUFFER_PRINT sections) unconditionally instead of the old version? The source would look nicer and the resulting image probably smaller.
I' ll aggree with you. Is this is ok for all?
Some nitpicking comments below.
Signed-off-by: Heiko Schocher hs@denx.de
common/cmd_nvedit.c | 8 ++++++++ include/configs/mgcoge.h | 1 + include/configs/mgsuvd.h | 1 + 3 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c index 85025da..8f6310a 100644 --- a/common/cmd_nvedit.c +++ b/common/cmd_nvedit.c @@ -89,8 +89,12 @@ int do_printenv (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) for (i=0; env_get_char(i) != '\0'; i=nxt+1) { for (nxt=i; env_get_char(nxt) != '\0'; ++nxt) ; +#if defined(CONFIG_ENV_BUFFER_PRINT)
puts ((char *)( env_get_addr(nxt) + 1));
Please stick with one coding-style rule, either "func ()" or "func()". Since this file already uses the "func()" style I suggest to use this version.
OK.
+#else for (k=i; k<nxt; ++k) putc(env_get_char(k)); +#endif putc ('\n');
Why not fold this "putc()" into the puts() above (replace it with printf)?
I make it in a printf (Think there was no reason for not doing it so)
thanks bye Heiko

Dear Heiko Schocher,
In message 49803324.3000709@denx.de you wrote:
Why not fold this "putc()" into the puts() above (replace it with printf)?
I make it in a printf (Think there was no reason for not doing it so)
Check if printf() is not printing a series of single characters again! Also, we tend to prefer using puts() / putc() over printf() because of the huge performance overhead involved in printf() when only plain constant strings need to be printed.
Best regards,
Wolfgang Denk

On Wednesday 28 January 2009, Wolfgang Denk wrote:
Why not fold this "putc()" into the puts() above (replace it with printf)?
I make it in a printf (Think there was no reason for not doing it so)
Check if printf() is not printing a series of single characters again! Also, we tend to prefer using puts() / putc() over printf() because of the huge performance overhead involved in printf() when only plain constant strings need to be printed.
printf() is already used in the 2nd part of the patch. But the 1st string can also be concat-ed of course and printed via puts(). printf() would be easier to read though from my point of view.
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================

Dear Stefan Roese,
In message 200901281256.14264.sr@denx.de you wrote:
Check if printf() is not printing a series of single characters again! Also, we tend to prefer using puts() / putc() over printf() because of the huge performance overhead involved in printf() when only plain constant strings need to be printed.
printf() is already used in the 2nd part of the patch. But the 1st string can also be concat-ed of course and printed via puts(). printf() would be easier to read though from my point of view.
Just because you are slow in one place (where you need the formatting capabilities of printf()) is no excuse for being slow in other parts (where you don't need it).
Best regards,
Wolfgang Denk
participants (3)
-
Heiko Schocher
-
Stefan Roese
-
Wolfgang Denk