All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefano Babic <sbabic@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] Tools: set multiple variable with fw_setenv utility
Date: Fri, 21 May 2010 18:39:00 +0200	[thread overview]
Message-ID: <4BF6B724.6090307@denx.de> (raw)
In-Reply-To: <20100521160351.CA8B3E67644@gemini.denx.de>

Wolfgang Denk wrote:

> Adding spaces to the variable value? This makes little sense to me.
> It's just a waste of storage space and boot time.

Well, I can agree with you, however I have already seen this case...

> If you feel your environment is so complicated to read that you want
> such "formatting", then rather structure your envrionment (see
> proposals on the list), and/or help improving the printenv
> capabilities to add sorting etc.

Ok, understood. I will change to support something like

<whitespaces>variable<whitespaces>value

> Who says that TAB would not be allowed?

..probably is more stranger as to have spaces....

> Of course it is. There is
> virtually no restriction on the content of the value of a variable.
> The only character that cannot be part of the value is '\0'.
> 
> [Of course it is not a wise decision to add non-printing or control
> characters, but you can do this, if you like.]

As I saw some leading whitespaces in a variable, I cannot remember a
case where I saw a TAB in a variable. But as it is not forbidden,
probably there is still this case !

> 
>>>> +	char dump[128];
>>> Ouch! That's short! Why do we need such an arbitrary limit?
>> We do not need a small value, but I have to set a value for fgets. A
>> bigger value should be enough, reporting an error if the line is too long.
> 
> But your code is not set up to handle the remainder of too long lines.
> It would be read as the next line and cause confusion.

Ah, ok, I get now the point.

> Why do you have to run this in two spearate passes at all - first
> scan, then process.
> 
> Why cannot you process each variable when you read it? The we would
> not need any array or list at all.

There is a reason, please tell me how good it is ;-)

I thought the code could be used not only as it is, but integrated in an
application linking the required object (only one, fw_env.o).
An application could internally generate a list of pairs variable/values
and needs a function to set them in the u-boot environment. This is the
reason of the int fw_setenv_multiple(fw_env_list *list, int count)
function. No need to use an external file, no need to call fw_setenv as
external process. An application could link fw_env.o and call
fw_setenv_multiple().

I agree with you that I can do everything in a single pass if we do not
provide such kind of external interface.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

  reply	other threads:[~2010-05-21 16:39 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-21 10:52 [U-Boot] [PATCH] Tools: set multiple variable with fw_setenv utility Stefano Babic
2010-05-21 11:38 ` Wolfgang Denk
2010-05-21 15:15   ` Stefano Babic
2010-05-21 16:03     ` Wolfgang Denk
2010-05-21 16:39       ` Stefano Babic [this message]
2010-05-21 19:11         ` Wolfgang Denk
2010-05-23 14:29 ` [U-Boot] [PATCH V2] " Stefano Babic
2010-05-24  2:47   ` Peter Tyser
2010-05-24  8:18     ` Stefano Babic
2010-05-24 10:08   ` [U-Boot] [PATCH V3] " Stefano Babic
2010-06-29 20:43     ` 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=4BF6B724.6090307@denx.de \
    --to=sbabic@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.