[U-Boot] [PATCH 1/2] env: set individual variables to default

[Wolfgang Denk]

Dear Gerlando Falauto,

In message <1316703972-8417-2-git-send-email-gerlando.falauto at keymile.com> you wrote:
> implement command "env default <vars>..." for resetting individual
> variables to their default values.
> 
> "env default -f" will always keep resetting the whole environment
> to default.

I don't think this is a good idea.

The "-f" should be used to allow resetting write-protected variables
like serial# or ethaddr.  With no variable names given, it will
therefore always be needed (assuming you have such protected variables
in your environment).  But it should also work when arguments are
given, i. e.

	env default ethaddr

is supposed to fail, because ethaddr is write protected.  To reset
this, you should use

	env default -f ethaddr

> +#ifdef CONFIG_CMD_DEFAULTENV_VARS
> +/*
> + * import individual variables from an external environment
> + * (e.g. default environment).
> + * Most of this code comes straight from himport_r().

Indeed.  This is way too much code copied.  Please factor this out
into a separate function to avoid such duplication (which is always a
maintenance nightmare).

...
> +			/* deal with "name" and "name=" entries (delete var) */
> +			if (*dp == '\0' || *(dp + 1) == '\0' ||
> +				*dp == sep || *(dp + 1) == sep) {
> +				if (*dp == '=')
> +					*dp++ = '\0';
> +				*dp++ = '\0';	/* terminate name */

Here the comment (about the "delete var") is even wrong.

> +		/*
> +		 * size check needed for text without '\0' termination
> +		 * (e.g. default environment)
> +		 */

What makes you think the default environment was NOT '\0' terminated?

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
It became apparent that one reason why the Ice Giants were  known  as
the  Ice  Giants  was  because they were, well, giants. The other was
that they were made of ice.              -Terry Pratchett, _Sourcery_