All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Warren <swarren@wwwdotorg.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] cmd_nvedit.c: setenv_hex must prefix hex with '0x'
Date: Fri, 04 Oct 2013 15:47:24 -0600	[thread overview]
Message-ID: <524F376C.7070703@wwwdotorg.org> (raw)
In-Reply-To: <20131004213519.75C4E38040E@gemini.denx.de>

On 10/04/2013 03:35 PM, Wolfgang Denk wrote:
> Dear Tom Rini,
> 
> In message <1380901758-30360-1-git-send-email-trini@ti.com> you wrote:
>> setenv_hex is only called with hex values, but does not prefix the
>> strings with '0x', as in general U-Boot assumes hex values not decimal
>> values, and this saves space in the environment.  However, some
>> functions such as 'load' take some values that are most easily described
>> in hex (load address) and decimal (size, offset within a file).
>>
>> This can lead to the situation where, for example, spl export is run,
>> which leads to a call of setenv_hex of the fdtaddr, which will be re-set
>> in the environment.  Then 'saveenv' may be run (after updating other
>> parts of the environment for falcon mode), causing an invalid for 'load'
>> fdtaddr to be saved to the environment and leading to future boots to
>> fail if using 'load' to read the fdt file.
> 
> I think this is not a correct way to fix the issues at hand.
> 
> All U-Boot (with the single unfortunate exception of the "sleep"
> command) defaults to hex input - that's what's documented, and what
> people have always been using for many, many years.  Applying your
> patch means that we modify just a single use case, while setting the
> same environment variables from the command line (without the leading
> 0x) would still trigger a problem.
> 
> I understand that the actual cause of these issues is commit 3f83c87
> "fs: fix number base behaviour change in fatload/ext*load".  Reviewing
> this patch I think that it should have never been applied.  The fact
> that  "load" and "fatload" / "ext*load" behave differently are reason
> enough to reject this patch.  "load" should just behave like every
> other command, and default to hex input.
> 
> I think we should NAK your patch, and suggest to fix the problem by
> reverting commit 3f83c87 and making "load" default to hex input mode.

Reverting 3f83c87 would do the opposite of what you want; it'd make
extload/fatload require 0x prefixes instead of assuming hex. Perhaps
what you want is a tweak to that patch so that the generic load/ls
commands always expect a hex value, rather than requiring the 0x prefix?

  reply	other threads:[~2013-10-04 21:47 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-04 15:49 [U-Boot] [PATCH] cmd_nvedit.c: setenv_hex must prefix hex with '0x' Tom Rini
2013-10-04 21:35 ` Wolfgang Denk
2013-10-04 21:47   ` Stephen Warren [this message]
2013-10-04 22:12     ` Wolfgang Denk
2013-10-04 22:30       ` Stephen Warren
2013-10-05 19:07 ` [U-Boot] [PATCH] Fix number base handling of "load" command Wolfgang Denk
2013-10-07 16:14   ` Stephen Warren
2013-10-07 19:40     ` Wolfgang Denk
2013-10-07 19:42   ` [U-Boot] [PATCH V2] " Wolfgang Denk
2013-10-07 20:04   ` [U-Boot] [PATCH] " Tom Rini

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=524F376C.7070703@wwwdotorg.org \
    --to=swarren@wwwdotorg.org \
    --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.