[U-Boot] [PATCH 5/5] New implementation for internal handling of environment variables.

[Matthias Fuchs]

Hi Wolfgang,

On Wednesday 28 July 2010 23:17, Wolfgang Denk wrote:
> Dear Matthias Fuchs,
> 
> In message <201007261652.39368.matthias.fuchs at esd-electronics.com> you wrote:
> > 
> > I could think of some situations where the new env command
> > is helpful. But more during development than for production systems.
> 
> It depends. "Reset to factory defaults" is a not so uncommon request.
> And acceleration of scripts is not so uncommon either.
> 
> > Switching between environment profiles would be cool. And a "env default -f"
> > behavior that keeps MAC addresses and serial# is also on my wishlist.
> 
> Actually neither MAC addresses nor  serial# are part of the default
> environment.
Right. But, I'd like to have that described feature in any case. Let's call it different,
but the functionality would be helpful. What about a further option like '-p' for keep
_p_rotected. 
> 
> > I did some testing on our PMC440 with environment in EEPROM.
> > Please see some comments below.
> 
> Thanks.
> 
> > > +static int do_env_export(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> > > +{
> > > +	char	buf[32];
> > > +	char	*addr, *cmd, *res;
> > > +	size_t	size;
> > > +	ssize_t	len;
> > > +	env_t	*envp = (env_t *)addr;
> > addr is uninitialized. declaration is enough here.
> 
> Will check this.
> 
> ...
> > > +	if (chk) {		/* export as checksum protected block */
> > Add:
> > 		envp = (env_t *)addr;
> > > +		res = (char *)&envp->data;
> > > +	} else {		/* export as raw binary data */
> > > +		res = (char *)&addr;
> > Should'n this be 
> > 		res = addr;
> 
> No. We need the address of the pointer variable, so the function can
> store the result pointer there.
Nak. You added &-operator when calling: hexport('\0', &res, ENV_SIZE);
That's one to much. You can also remove the &-operator from this:
		res = (char *)&envp->data;

So the code will look like this:

	if (chk) {		/* export as checksum protected block */
		envp = (env_t *)addr;
		res = (char *)envp->data;
	} else {		/* export as raw binary data */
		res = addr;
	}

	len = hexport('\0', &res, ENV_SIZE);
...

Matthias