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

[Matthias Fuchs <matthias.fuchs@esd-electronics.com>]

Hi Wolfgang,

I could think of some situations where the new env command
is helpful. But more during development than for production systems.

Switching between environment profiles would be cool. And a "env default -f"
behavior that keeps MAC addresses and serial# is also on my wishlist.

I did some testing on our PMC440 with environment in EEPROM.
Please see some comments below.

On Saturday 17 July 2010 21:45, Wolfgang Denk wrote:
> diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
> index 8c86f15..976a30b 100644
> --- a/common/cmd_nvedit.c
> +++ b/common/cmd_nvedit.c
...
> +/*
> + * env export [-t | -b | -c] addr [size]
> + *	-t:	export as text format; if size is given, data will be
> + *		padded with '\0' bytes; if not, one terminating '\0'
> + *		will be added (which is included in the "filesize"
> + *		setting so you can for exmple copy this to flash and
> + *		keep the termination).
> + *	-b:	export as binary format (name=value pairs separated by
> + *		'\0', list end marked by double "\0\0")
> + *	-c:	export as checksum protected environment format as
> + *		used for example by "saveenv" command
> + *	addr:	memory address where environment gets stored
> + *	size:	size of output buffer
> + *
...
> +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.
> +	char	sep = '\n';
> +	int	chk = 0;
> +	int	fmt = 0;
> +
> +	cmd = *argv;
> +
> +	while (--argc > 0 && **++argv == '-') {
I'd like some more braces for readability.

> +		char *arg = *argv;
> +		while (*++arg) {
> +			switch (*arg) {
> +			case 'b':		/* raw binary format */
> +				if (fmt++)
> +					goto sep_err;
> +				sep = '\0';
> +				break;
> +			case 'c':		/* external checksum format */
> +				if (fmt++)
> +					goto sep_err;
> +				sep = '\0';
> +				chk = 1;
> +				break;
> +			case 't':		/* text format */
> +				if (fmt++)
> +					goto sep_err;
> +				sep = '\n';
> +				break;
> +			default:
> +				cmd_usage(cmdtp);
> +				return 1;
> +			}
> +		}
> +	}
>  
> -/**************************************************/
> +	if (argc < 1) {
> +		cmd_usage(cmdtp);
> +		return 1;
> +	}
> +
> +	addr = (char *)simple_strtoul(argv[0], NULL, 16);
> +
> +	if (argc == 2) {
> +		size = simple_strtoul(argv[1], NULL, 16);
> +		memset(addr, '\0', size);
> +	} else {
> +		size = 0;
> +	}
> +
> +	if (sep) {		/* export as text file */
> +		len = hexport(sep, &addr, size);
> +		if (len < 0) {
> +			error("Cannot export environment: errno = %d\n",
> +				errno);
> +			return 1;
> +		}
> +		sprintf(buf, "%zX", len);
> +		setenv("filesize", buf);
> +
> +		return 0;
> +	}
> +
> +	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;

> +	}
> +
> +	len = hexport('\0', &res, ENV_SIZE);
> +	if (len < 0) {
> +		error("Cannot export environment: errno = %d\n",
> +			errno);
> +		return 1;
> +	}
> +
> +	if (chk) {
> +		envp->crc   = crc32(0, envp->data, ENV_SIZE);
> +#ifdef CONFIG_ENV_ADDR_REDUND
> +		envp->flags = ACTIVE_FLAG;
> +#endif
> +	}
> +	sprintf(buf, "%zX", len + offsetof(env_t,data));
> +	setenv("filesize", buf);
> +
> +	return 0;
> +
> +sep_err:
> +	printf("## %s: only one of \"-b\", \"-c\" or \"-t\" allowed\n",
> +		cmd);
> +	return 1;
> +}
> +
...


Fixes for non-building board (AR405, CANBT, PMC440) will come up shortly.

Matthias