All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wolfgang Denk <wd@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v3 01/10] Add getenv_ulong() to read an integer from an environment variable
Date: Fri, 14 Oct 2011 23:42:16 +0200	[thread overview]
Message-ID: <20111014214216.0461714094B2@gemini.denx.de> (raw)
In-Reply-To: <1318626284-11161-1-git-send-email-sjg@chromium.org>

Dear Simon Glass,

In message <1318626284-11161-1-git-send-email-sjg@chromium.org> you wrote:
> This is not an uncommon operation in U-Boot, so let's put it in a common
> function.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
...
> +ulong getenv_ulong(const char *name, int base, ulong default_val)
> +{
> +	char buff[20];
> +	const char *str = NULL;
> +
> +	/*
> +	 * Prior to the import of the environment into the hashtable we
> +	 * should not call getenv()
> +	 */
> +	if (gd->flags & GD_FLG_ENV_READY)
> +		str = getenv(name);
> +	else if (getenv_f(name, buff, sizeof(buff)) > 0)
> +		str = buff;
> +	return str ? simple_strtoul(str, NULL, base) : default_val;
> +}

Sorry, I just changed my mind.

The issue with using getenv() before relocation is that it uses just a
tiny buffer (usually 32 bytes) in the global data structure to store
the result, which is often not sufficient for user provided data (some
boards have hwconfig strings that are _much_ longer than that, and
only the caller knows what to expect).

It's only now that I realize that we are dealing with int / long
values here only, so the length of the expected strings is indeed
limited - actually the buffer you provide here is way smaller that
what getenv() uses.

Can we please switch back to the previous version, but insert a
comment that explains why getenv_f() is not needed in this specific
case?

Thanks, and apologies for causing additional efforts.

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
Status quo. Latin for "the mess we're in."

  reply	other threads:[~2011-10-14 21:42 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-14 16:48 [U-Boot] [PATCH v2 01/10] Add getenv_ulong() to read an integer from an environment variable Simon Glass
2011-10-14 17:09 ` Mike Frysinger
2011-10-14 19:36 ` Wolfgang Denk
2011-10-14 21:04 ` [U-Boot] [PATCH v3 " Simon Glass
2011-10-14 21:42   ` Wolfgang Denk [this message]
2011-10-14 23:27     ` Simon Glass
2011-10-14 23:25 ` [U-Boot] [PATCH v4 " Simon Glass
2011-10-15 17:21   ` Mike Frysinger
2011-10-23 20:49   ` Wolfgang Denk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20111014214216.0461714094B2@gemini.denx.de \
    --to=wd@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.