[U-Boot] [PATCH] make fw_setenv write multiple variables
This patch makes fw_setenv write multiple variables on a single command line. That is "fw_setenv var1 value1 var2 value2" will write all variables and values without the need to erase multiple times the environment.
regards, Nikos
Dear Nikos Mavrogiannopoulos,
In message 49F9738D.9060703@gennetsa.com you wrote:
This patch makes fw_setenv write multiple variables on a single command line. That is "fw_setenv var1 value1 var2 value2" will write all variables and values without the need to erase multiple times the environment.
I reject this as it would introduce serious incompatibilities with the existing interfaces, both in fw_setenv and in the U-Boot setenv command.
Not to mention that it would be extremely error prone.
But I understand that such a functionality may be useful - just your interface design is bogus.
What about the following alternative: add an option to fw_setenv to read the commands (evventually even with an [optional?] leading "setenv") from a file (or from stdin, if no file name or '-' is given). Then you can have one set of parameters per line, and no argument counting or similar things are needed.
What do you think?
Best regards,
Wolfgang Denk
This patch makes fw_setenv write multiple variables on a single command line. That is "fw_setenv var1 value1 var2 value2" will write all variables and values without the need to erase multiple times the environment.
IIRC fw_setenv uses the first word as key and all the rest as the value. Judging on the description of your patch, this will probably break this behaviour.
I wrote a fw_editenv with -k/-v flags a couple of years ago in order to get around this, but i don't remember if I submitted it to the list.
If I didn't in the past, I did now; this is what we are using to write from the firmware to do bulk changes in the u-boot config space.
The patch is against 1.3.3
Dear Marc,
In message 20090430113741.GD10108@crichton.homelinux.org you wrote:
If I didn't in the past, I did now; this is what we are using to write from the firmware to do bulk changes in the u-boot config space.
The patch is against 1.3.3
Thanks a lot.
Hm... this seems a lot of code to deal with a trivial issue. I think implementing my idea (reading the input line by line from a text file) would be much easier to implement and easier to use as well.
And of course the patch doesn't apply any more against current code.
[Not to mention that the Signed-off-by: line is missing, plus there are coding style issues.]
Best regards,
Wolfgang Denk
Hm... this seems a lot of code to deal with a trivial issue. I think implementing my idea (reading the input line by line from a text file) would be much easier to implement and easier to use as well.
Yeah, probably.
IIRC, I tried to stick as close to the fw_Xenv code structure as possible; as such, this basically copies the fw_setenv block and modifies it without interfering or breaking in the upstream code.
And of course the patch doesn't apply any more against current code.
I haven't looked at it, but unless there are large differences and rewrites in fw_env.c; the block of code should apply pretty easily: that's one of the underlying reasons keeping to current fw_env.c code separate and adding on it.
[Not to mention that the Signed-off-by: line is missing, plus there are coding style issues.]
I didn't expect it to get applied: I just used it as input to the
fw_setenv key val val val val
example
Dear Marc Leeman,
In message 1f729c480904300627r5661774ej7d2cf5a19bee1083@mail.gmail.com you wrote:
IIRC, I tried to stick as close to the fw_Xenv code structure as possible; as such, this basically copies the fw_setenv block and modifies it without interfering or breaking in the upstream code.
Indeed. That code duplication is another problem, which for itself would be reason enough to reject the patch. It's unmaintainable.
Best regards,
Wolfgang Denk
participants (3)
-
Marc Leeman -
Nikos Mavrogiannopoulos -
Wolfgang Denk