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

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

[Heiko Schocher]

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
-- 
1.6.0.6

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

[Stefan Roese]

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 at denx.de
=====================================================================

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

[Heiko Schocher]

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
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

[Wolfgang Denk]

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

-- 
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
Sometimes a man will tell his bartender things he'll never tell his doctor.
	-- Dr. Phillip Boyce, "The Menagerie" ("The Cage"),
	   stardate unknown.

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

[Stefan Roese]

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 at denx.de
=====================================================================

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

[Wolfgang Denk]

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

-- 
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
"The two most common things in the universe are hydrogen  and  stupi-
dity."