Remove code duplication for setting the default environment

PATCH New Harald Welte 5 participants 9 messages Jul 6, 2008 – Jul 10, 2008

lore

Share & tools

lore

Harald Welte

author

2008-07-06 17:06 UTC #0 lore raw

PATCH 1/1 Remove code duplication for setting the default environment

Remove code duplication for setting the default environment

common/env_common.c (default_env): new function that resets the environment to
the default value
common/env_common.c (env_relocate): use default_env instead of own copy common/env_nand.c (env_relocate_spec): use default_env instead of own copy include/environment.h: added default_env prototype

Signed-off-by Werner Almesberger <[email protected]>

Signed-off-by Harald Welte <[email protected]>

common/env_common.c +20 ~~-16~~

Index: u-boot/common/env_common.c
===================================================================
@@ -192,6 +192,25 @@
 	}
 }
 
+void default_env(void)
+{
+	if (sizeof(default_environment) > ENV_SIZE)
+	{
+		puts ("*** Error - default environment is too large\n\n");
+		return;
+	}
+
+	memset (env_ptr, 0, sizeof(env_t));
+	memcpy (env_ptr->data,
+		default_environment,
+		sizeof(default_environment));
+#ifdef CFG_REDUNDAND_ENVIRONMENT
+	env_ptr->flags = 0xFF;
+#endif
+	env_crc_update ();
+	gd->env_valid = 1;
+}
+
 void env_relocate (void)
 {
 	DEBUGF ("%s[%d] offset = 0x%lx\n", __FUNCTION__,__LINE__,

@@ -228,22 +247,7 @@
 		puts ("*** Warning - bad CRC, using default environment\n\n");
 		show_boot_progress (-60);
 #endif
-
-		if (sizeof(default_environment) > ENV_SIZE)
-		{
-			puts ("*** Error - default environment is too large\n\n");
-			return;
-		}
-
-		memset (env_ptr, 0, sizeof(env_t));
-		memcpy (env_ptr->data,
-			default_environment,
-			sizeof(default_environment));
-#ifdef CFG_REDUNDAND_ENVIRONMENT
-		env_ptr->flags = 0xFF;
-#endif
-		env_crc_update ();
-		gd->env_valid = 1;
+		default_env();
 	}
 	else {
 		env_relocate_spec ();

common/env_nand.c +1 ~~-13~~

Index: u-boot/common/env_nand.c
===================================================================
@@ -363,19 +363,7 @@
 static void use_default()
 {
 	puts ("*** Warning - bad CRC or NAND, using default environment\n\n");
-
-	if (default_environment_size > CFG_ENV_SIZE){
-		puts ("*** Error - default environment is too large\n\n");
-		return;
-	}
-
-	memset (env_ptr, 0, sizeof(env_t));
-	memcpy (env_ptr->data,
-			default_environment,
-			default_environment_size);
-	env_ptr->crc = crc32(0, env_ptr->data, ENV_SIZE);
-	gd->env_valid = 1;
-
+	default_env();
 }
 #endif

include/environment.h +3 -0

Index: u-boot/include/environment.h
===================================================================
@@ -117,4 +117,7 @@
 /* Function that updates CRC of the enironment */
 void env_crc_update (void);
 
+
+void default_env(void);
+
 #endif	/* _ENVIRONMENT_H_ */

Signature · 11 lines

- Harald Welte <[email protected]>           http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080707/4de94f90/attachment.pgp

Wolfgang Denk

2008-07-06 18:57 UTC #1 lore raw

PATCH 1/1 Remove code duplication for setting the default environment

common/env_common.c line 201

Quoted 8 lines · In message <[email protected]> you wrote:

common/env_common.c +7 -0

+void default_env(void)
+{
+	if (sizeof(default_environment) > ENV_SIZE)
+	{
+		puts ("*** Error - default environment is too large\n\n");
+		return;
+	}

Incorrect brace style.

common/env_common.c line 206

Quoted 4 lines

common/env_common.c +4 -0

+	memset (env_ptr, 0, sizeof(env_t));
+	memcpy (env_ptr->data,
+		default_environment,
+		sizeof(default_environment));

Put on one line ?

Quoted 2 lines

common/env_common.c +0 -0

...

Quoted 1 line

common/env_common.c +0 -1

-		if (sizeof(default_environment) > ENV_SIZE)

...

Quoted 2 lines

common/env_nand.c +0 -0

...

common/env_nand.c line 367

Quoted 1 line

common/env_nand.c +0 -1

-	if (default_environment_size > CFG_ENV_SIZE){

Looks like a sleeping bug to me...

Quoted 1 line

+void default_env(void);

Please name "set_default_env()".

Best regards,

Wolfgang Denk

Signature · 4 lines

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
Genitiv ins Wasser, weil's Dativ ist!

Harald Welte

author

2008-07-07 07:40 UTC #2 lore raw

PATCH 1/1 Remove code duplication for setting the default environment

New patch at the end of this mail!

Quoted 11 lines · Wolfgang Denk

Review
Quoted 8 lines · In message <[email protected]> you wrote:
(quoted diff) +7 -0
+void default_env(void)
+{
+	if (sizeof(default_environment) > ENV_SIZE)
+	{
+		puts ("*** Error - default environment is too large\n\n");
+		return;
+	}
Incorrect brace style.

just moving one piece of code around here. Didn't want to introduce more whitespace changes than needed in order to make clear there is no functional change, but just code reorganization. Changed it now.

Quoted 6 lines

Review

Quoted 4 lines

(quoted diff) +4 -0

+	memset (env_ptr, 0, sizeof(env_t));
+	memcpy (env_ptr->data,
+		default_environment,
+		sizeof(default_environment));

Put on one line ?

too long for one line, but works on two lines.

Quoted 11 lines

Quoted 2 lines
common/env_common.c +0 -0
...
Quoted 1 line
common/env_common.c +0 -1
-		if (sizeof(default_environment) > ENV_SIZE)
...
Quoted 2 lines
common/env_nand.c +0 -0
...
common/env_nand.c
Quoted 1 line
common/env_nand.c +0 -1
-	if (default_environment_size > CFG_ENV_SIZE){
Looks like a sleeping bug to me...

yes, indeed. should have been ENV_SIZE before... with the old code you can overflow env_ptr->data.

Quoted 3 lines

Quoted 1 line
+void default_env(void);
Please name "set_default_env()".

done.

***********************

Remove code duplication for setting the default environment

Signed-off-by Werner Almesberger <[email protected]>

Signed-off-by Harald Welte <[email protected]>

common/env_common.c +18 ~~-16~~

Index: u-boot/common/env_common.c
===================================================================
@@ -192,6 +192,23 @@
 	}
 }
 
+void set_default_env(void)
+{
+	if (sizeof(default_environment) > ENV_SIZE) {
+		puts ("*** Error - default environment is too large\n\n");
+		return;
+	}
+
+	memset(env_ptr, 0, sizeof(env_t));
+	memcpy(env_ptr->data, default_environment,
+	       sizeof(default_environment));
+#ifdef CFG_REDUNDAND_ENVIRONMENT
+	env_ptr->flags = 0xFF;
+#endif
+	env_crc_update ();
+	gd->env_valid = 1;
+}
+
 void env_relocate (void)
 {
 	DEBUGF ("%s[%d] offset = 0x%lx\n", __FUNCTION__,__LINE__,

@@ -228,22 +245,7 @@
 		puts ("*** Warning - bad CRC, using default environment\n\n");
 		show_boot_progress (-60);
 #endif
-
-		if (sizeof(default_environment) > ENV_SIZE)
-		{
-			puts ("*** Error - default environment is too large\n\n");
-			return;
-		}
-
-		memset (env_ptr, 0, sizeof(env_t));
-		memcpy (env_ptr->data,
-			default_environment,
-			sizeof(default_environment));
-#ifdef CFG_REDUNDAND_ENVIRONMENT
-		env_ptr->flags = 0xFF;
-#endif
-		env_crc_update ();
-		gd->env_valid = 1;
+		set_default_env();
 	}
 	else {
 		env_relocate_spec ();

common/env_nand.c +1 ~~-13~~

Index: u-boot/common/env_nand.c
===================================================================
@@ -363,19 +363,7 @@
 static void use_default()
 {
 	puts ("*** Warning - bad CRC or NAND, using default environment\n\n");
-
-	if (default_environment_size > CFG_ENV_SIZE){
-		puts ("*** Error - default environment is too large\n\n");
-		return;
-	}
-
-	memset (env_ptr, 0, sizeof(env_t));
-	memcpy (env_ptr->data,
-			default_environment,
-			default_environment_size);
-	env_ptr->crc = crc32(0, env_ptr->data, ENV_SIZE);
-	gd->env_valid = 1;
-
+	set_default_env();
 }
 #endif

include/environment.h +3 -0

Index: u-boot/include/environment.h
===================================================================
@@ -117,4 +117,7 @@
 /* Function that updates CRC of the enironment */
 void env_crc_update (void);
 
+/* [re]set to the default environment */
+void set_default_env(void);
+
 #endif	/* _ENVIRONMENT_H_ */

Signature · 11 lines

- Harald Welte <[email protected]>           http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080707/f4d24aa0/attachment.pgp

Wolfgang Denk

Applied

2008-07-09 22:30 UTC #3 lore raw

PATCH 1/1 Remove code duplication for setting the default environment

Quoted 2 lines · In message <[email protected]> you wrote:

New patch at the end of this mail!

Could you please start using git to prepare and send patches? That would save me (and others) a lot of time.

Thanks in advance.

Quoted 8 lines

Remove code duplication for setting the default environment
common/env_common.c (default_env): new function that resets the environment to
the default value
common/env_common.c (env_relocate): use default_env instead of own copy common/env_nand.c (env_relocate_spec): use default_env instead of own copy include/environment.h: added default_env prototype

Quoted 4 lines

Signed-off-by Werner Almesberger <[email protected]>
Signed-off-by Harald Welte <[email protected]>
Index: u-boot/common/env_common.c

Applied, thanks.

Best regards,

Wolfgang Denk

Signature · 7 lines

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
You young'uns. That was *long* before AltaVista,  DejaNews,  or  even
(gasp) the *Web*! In fact, we typed that thread on steam-powered card
punchers, and shipped it around via Pony Express.
            -- Randal Schwartz in <[email protected]>

Harald Welte

author

2008-07-10 00:21 UTC #4 lore raw

[U-Boot-Users] usage of git to send patches to u-boot mailinglist

Quoted 6 lines · Wolfgang Denk

Quoted 2 lines · In message <[email protected]> you wrote:
New patch at the end of this mail!
Could you please start using git to prepare and send patches? That would save me (and others) a lot of time.

can do, even though I believe it is by far not the best tool to do so. The problem is that I would have to use one local branch per feature (i.e. lots of local branches that need to be kept in sync), and even then any incremental changes/fixes to one particular feature are visible in the commitlog (and thus result in changelog pollution).

My best experience so far really is quilt for maintaining patchsets. You can keep a large number of patches, easily switch between them and keep your modifications organized per-feature, rather than in the chronological commit order of a revision control system.

So what I can probably do is to continue to use quilt up to the point where I'd want to send something to a mailinglist, and then put into a local git branch, export the patch from there and send it to the list.

However, any further change to that patch based on feedback from the list would again go into the quilt tree, I'd have to start with a clean 'origin' u-boot git tree and commit the modified change into the git
tree. Otherwise we start having all the commit messages (like 'changed
coding style according to mailinglist feedback') in the code, even _before_ that code was ever merged into the respective mainline git tree.

So is this really the preferred workflow? How are others dealing with this? How to avoid commitlog pollution?

Quoted 1 line

Applied, thanks.

thanks!

Signature · 11 lines

- Harald Welte <[email protected]>           http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080710/852ce08e/attachment.pgp

Paul Gortmaker

2008-07-10 04:36 UTC #5 lore raw

[U-Boot-Users] usage of git to send patches to u-boot mailinglist

Quoted 31 lines · Harald Welte

Quoted 6 lines · Wolfgang Denk
Quoted 2 lines · In message <[email protected]> you wrote:
New patch at the end of this mail!
Could you please start using git to prepare and send patches? That would save me (and others) a lot of time.
can do, even though I believe it is by far not the best tool to do so. The problem is that I would have to use one local branch per feature (i.e. lots of local branches that need to be kept in sync), and even then any incremental changes/fixes to one particular feature are visible in the commitlog (and thus result in changelog pollution).
My best experience so far really is quilt for maintaining patchsets. You can keep a large number of patches, easily switch between them and keep your modifications organized per-feature, rather than in the chronological commit order of a revision control system.
So what I can probably do is to continue to use quilt up to the point where I'd want to send something to a mailinglist, and then put into a local git branch, export the patch from there and send it to the list.
However, any further change to that patch based on feedback from the list would again go into the quilt tree, I'd have to start with a clean 'origin' u-boot git tree and commit the modified change into the git
tree. Otherwise we start having all the commit messages (like 'changed
coding style according to mailinglist feedback') in the code, even _before_ that code was ever merged into the respective mainline git tree.

Quoted 2 lines

So is this really the preferred workflow? How are others dealing with this? How to avoid commitlog pollution?

If you really like quilt, then perhaps look into guilt? (A cross between the functionality of quilt, layered onto the underpinnings of git).

Or you can cherry-pick your unchanged commits onto a new branch, plus the one requiring a change; make the change, do a git-add and a git-commit --amend and then cherry pick the remaining unchanged commits. (I believe this is essentially what guilt would do for you.)

The idea here is that since you aren't expecting someone else to directly pull from your git repo (i.e. if you aren't a main maintainer) then you are free to "rebase" and re-write history to fix your mistakes without having to continually add on new commits and thus get commit log pollution.

I don't claim to be a git expert, but I hope that helps some. You might find this an interesting read too.

http://kerneltrap.org/Linux/Git_Management

Paul.

Quoted 30 lines

Quoted 2 lines
Applied, thanks.
thanks!
--
- Harald Welte <[email protected]> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                 (ETSI EN 300 175-7 Ch. A6)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
iD8DBQFIdVYcXaXGVTD0i/8RAsV/AJ0csYVQbIRTa7iqAf3BGtS0xQORlACeL25v AeUH4n0YtPWYvyig6S/16pA=
=JIP7
-----END PGP SIGNATURE-----
-------------------------------------------------------------------------
Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW! Studies have shown that voting for your favorite open source project, along with a healthy diet, reduces your potential for chronic lameness and boredom. Vote Now at http://www.sourceforge.net/community/cca08
_______________________________________________
U-Boot-Users mailing list
U-Boot-Users at lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/u-boot-users

Markus Klotzbücher

2008-07-10 08:34 UTC #6 lore raw

[U-Boot-Users] usage of git to send patches to u-boot mailinglist

In reply to Harald Welte

Hi Harald,

Quoted 23 lines · Harald Welte

can do, even though I believe it is by far not the best tool to do so. The problem is that I would have to use one local branch per feature (i.e. lots of local branches that need to be kept in sync), and even then any incremental changes/fixes to one particular feature are visible in the commitlog (and thus result in changelog pollution).
My best experience so far really is quilt for maintaining patchsets. You can keep a large number of patches, easily switch between them and keep your modifications organized per-feature, rather than in the chronological commit order of a revision control system.
So what I can probably do is to continue to use quilt up to the point where I'd want to send something to a mailinglist, and then put into a local git branch, export the patch from there and send it to the list.
However, any further change to that patch based on feedback from the list would again go into the quilt tree, I'd have to start with a clean 'origin' u-boot git tree and commit the modified change into the git
tree. Otherwise we start having all the commit messages (like 'changed
coding style according to mailinglist feedback') in the code, even _before_ that code was ever merged into the respective mainline git tree.

Quoted 2 lines

So is this really the preferred workflow? How are others dealing with this? How to avoid commitlog pollution?

I never used quilt, but I believe stacked git (stgit) implements more or less the same behavior on top of git.

Best regards

Markus Klotzbuecher

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

Stefan Roese

2008-07-10 08:49 UTC #7 lore raw

[U-Boot-Users] usage of git to send patches to u-boot mailinglist

Quoted 5 lines · Markus Klotzb?cher

Quoted 2 lines
So is this really the preferred workflow? How are others dealing with this? How to avoid commitlog pollution?
I never used quilt, but I believe stacked git (stgit) implements more or less the same behavior on top of git.

Yes, and guilt should be similar too. I'm looking forward to the guilt BOFS at OLS in a few weeks.

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

Wolfgang Denk

2008-07-10 09:00 UTC #8 lore raw

[U-Boot-Users] usage of git to send patches to u-boot mailinglist

In reply to Harald Welte

Quoted 2 lines · In message <[email protected]> you wrote:

can do, even though I believe it is by far not the best tool to do so.

The definition of "the best tool" depends on many things, including previous experience and personal preferences.

Quoted 4 lines

The problem is that I would have to use one local branch per feature (i.e. lots of local branches that need to be kept in sync), and even then any incremental changes/fixes to one particular feature are visible in the commitlog (and thus result in changelog pollution).

Having many local branches is no problem with git.

Git provides excellent help to rebase such branches, and using "--interactive" gives you a lot of options to edit the history.

Quoted 2 lines

So is this really the preferred workflow? How are others dealing with this? How to avoid commitlog pollution?

I started using "git-rebase -i", and so far it seems to work fine for me. But I'm definitely not an expert.

Best regards,

Wolfgang Denk

Signature · 4 lines

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
Contrary to popular belief, thinking does not cause brain damage.