[U-Boot-Users] [PATCH] Remove code duplication for setting the default environment

[U-Boot-Users] [PATCH] Remove code duplication for setting the default environment

[Harald Welte]

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_ */
-- 
- 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)
-------------- 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 

[U-Boot-Users] [PATCH] Remove code duplication for setting the default environment

[Wolfgang Denk]

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

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

[U-Boot-Users] [PATCH] Remove code duplication for setting the default environment

[Harald Welte]

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_ */
-- 
- 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)
-------------- 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 

[U-Boot-Users] [PATCH] Remove code duplication for setting the default environment

[Wolfgang Denk]

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

-- 
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 <8cwww1cd0d.fsf@gadget.cscaper.com>

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

[Harald Welte]

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!

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

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

[Paul Gortmaker]

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 at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/u-boot-users
>
>

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

[Markus Klotzbücher]

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

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

[Stefan Roese]

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

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

[Wolfgang Denk]

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

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