[U-Boot-Users] [PATCH] 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 werner@openmoko.org Signed-off-by: Harald Welte laforge@openmoko.org
Index: u-boot/common/env_common.c =================================================================== --- u-boot.orig/common/env_common.c +++ 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 (); Index: u-boot/common/env_nand.c =================================================================== --- u-boot.orig/common/env_nand.c +++ 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
Index: u-boot/include/environment.h =================================================================== --- u-boot.orig/include/environment.h +++ 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_ */
In message 20080706170626.GH20299@prithivi.gnumonks.org you wrote:
+void default_env(void) +{
- if (sizeof(default_environment) > ENV_SIZE)
- {
puts ("*** Error - default environment is too large\n\n");return;- }
Incorrect brace style.
- memset (env_ptr, 0, sizeof(env_t));
- memcpy (env_ptr->data,
default_environment,sizeof(default_environment));
Put on one line ?
--- u-boot.orig/common/env_common.c +++ u-boot/common/env_common.c
...
if (sizeof(default_environment) > ENV_SIZE)
...
--- u-boot.orig/common/env_nand.c +++ u-boot/common/env_nand.c
...
- if (default_environment_size > CFG_ENV_SIZE){
Looks like a sleeping bug to me...
+void default_env(void);
Please name "set_default_env()".
Best regards,
Wolfgang Denk
New patch at the end of this mail!
On Sun, Jul 06, 2008 at 08:57:34PM +0200, Wolfgang Denk wrote:
In message 20080706170626.GH20299@prithivi.gnumonks.org you wrote:
+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.
- 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.
--- u-boot.orig/common/env_common.c +++ u-boot/common/env_common.c
...
if (sizeof(default_environment) > ENV_SIZE)...
--- u-boot.orig/common/env_nand.c +++ u-boot/common/env_nand.c
...
- 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.
+void default_env(void);
Please name "set_default_env()".
done.
***********************
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 werner@openmoko.org Signed-off-by: Harald Welte laforge@openmoko.org
Index: u-boot/common/env_common.c =================================================================== --- u-boot.orig/common/env_common.c +++ 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 (); Index: u-boot/common/env_nand.c =================================================================== --- u-boot.orig/common/env_nand.c +++ 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
Index: u-boot/include/environment.h =================================================================== --- u-boot.orig/include/environment.h +++ 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_ */
In message 20080707074039.GC4412@prithivi.gnumonks.org 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.
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 werner@openmoko.org Signed-off-by: Harald Welte laforge@openmoko.org
Index: u-boot/common/env_common.c
Applied, thanks.
Best regards,
Wolfgang Denk
On Thu, Jul 10, 2008 at 12:30:03AM +0200, Wolfgang Denk wrote:
In message 20080707074039.GC4412@prithivi.gnumonks.org 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?
Applied, thanks.
thanks!
On Wed, Jul 9, 2008 at 8:21 PM, Harald Welte laforge@gnumonks.org wrote:
On Thu, Jul 10, 2008 at 12:30:03AM +0200, Wolfgang Denk wrote:
In message 20080707074039.GC4412@prithivi.gnumonks.org 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?
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.
Applied, thanks.
thanks!
--
- Harald Welte laforge@gnumonks.org 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@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/u-boot-users
Hi Harald,
Harald Welte laforge@gnumonks.org writes:
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?
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@denx.de
On Thursday 10 July 2008, Markus Klotzbücher wrote:
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@denx.de =====================================================================
In message 20080710002148.GD4657@prithivi.gnumonks.org 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.
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.
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
participants (5)
-
Harald Welte -
Markus Klotzbücher -
Paul Gortmaker -
Stefan Roese -
Wolfgang Denk