From: Wolfgang Denk <wd@denx.de>
Date: Fri, 04 Jan 2013 23:06:56 +0100
Subject: [U-Boot] [PATCH 6/7] getenv_f() env variable exist w/o needing
	a buffer
In-Reply-To: <1357323245-12455-6-git-send-email-yorksun@freescale.com>
References: <1357323245-12455-1-git-send-email-yorksun@freescale.com>
	<1357323245-12455-6-git-send-email-yorksun@freescale.com>
Message-ID: <20130104220656.EE90B202B9B@gemini.denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Dear York Sun,

In message <1357323245-12455-6-git-send-email-yorksun@freescale.com> you wrote:
> From: James Yang <James.Yang@freescale.com>
> 
> getenv_f() searches the environment for a variable name and copies the
> value of the variable to a buffer pointed to by one of the function's
> parameters.  However, this means that the buffer needs to exist and
> needs to be of sufficient length (passed as another parameter to
> getenv_f()) to hold the requested variable's value, even if all that is
> desired is the mere detection of the existence of the variable itself.
> 
> This patch removes the requirement that the buffer needs to exist.  If
> the pointer to the buffer is set to NULL and the requested variable is

Hm... this adds a special case and as such increases complexity - and
what is the benefit for you? 

In your code, you use this feature exactly once, which means all you
save is a single buffer on the stack of a function that does not
appear to be critical in terms of stack size.

>  /*
>   * Look up variable from environment for restricted C runtime env.
> + * If the variable is found, return the number of bytes copied.
> + * If buf is NULL, len is ignored, and, if the variable is found, return 1.
> + * If the variable is not found, return -1.

I think your description is not quite correct, and I dislike the
inconsistent behaviour we get though your patch.  So far, this
function returns the length of the variable value, or -1 in case of
errors.  This should not change even if we implement the suggested
feature, i. e. even when passing NULL as buffer pointer the function
should still return the length, and not some unrelated result.

> +		/* found */
> +		if (!buf)
> +			return 1;

I tend to NAK this part.

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
"One lawyer can steal more than a hundred men with guns."
- The Godfather