From: Wolfgang Denk <wd@denx.de>
To: Art Nikpal <email2tema@gmail.com>
Cc: Simon Glass <sjg@chromium.org>, Tom Rini <trini@konsulko.com>,
marek.behun@nic.cz, Neil Armstrong <narmstrong@baylibre.com>,
Tom Warren <twarren@nvidia.com>,
Andre Przywara <andre.przywara@arm.com>,
U-Boot Mailing List <u-boot@lists.denx.de>,
u-boot-amlogic@groups.io,
Christian Hewitt <christianshewitt@gmail.com>,
Artem Lapkin <art@khadas.com>, Nick Xie <nick@khadas.com>,
Gouwa Wang <gouwa@khadas.com>,
Francis Laniel <francis.laniel@amarulasolutions.com>
Subject: Re: [PATCH v2 0/2] env: setenv add resolve value option
Date: Sat, 20 Nov 2021 13:36:09 +0100 [thread overview]
Message-ID: <2235982.1637411769@gemini.denx.de> (raw)
In-Reply-To: <CAKaHn9KZg13r=pGCo=Lv69pBP-s-EW4uxjBVuH9veT+o5j9v+g@mail.gmail.com>
Dear Artem,
In message <CAKaHn9KZg13r=pGCo=Lv69pBP-s-EW4uxjBVuH9veT+o5j9v+g@mail.gmail.com> you wrote:
>
> next examples just demonstrate how its works for already defined env
> variables which contain other variables (like storred env variables)
Which next examples?
> sure I know about this ! see my prev message please .
Which exact message are you referring to here?
> Why not have this new opportunity ?
I think the suggested code is adding more problems than it solves.
> > have in mind, as you speak of "deep resolve". But then, I'm first
> > missing an explanation (and documentation) of what "deep resolve"
>
> recurrent resolving for variables
Your implementation of recursion has an arbiotrary and undocumented
depth limit. Also, I cannot see a way to prevent resolving in case I
want to keep something like "$foo" in the result.
But that's to be expected from such a non-standard way.
Why don't you stick with what "eval" in a standard shell does?
> > actually means in this context, i. e. how many levels down you
> > evaluat. Oh... the code has "int max_loop = 32;" - this is a
>
> i think its will be enough
It is a reallybad habt to implement code with arbitrary limits, as
it will blow into your face (or more likely that of an innocent
user) rather sooner than later. It's even worse that this limit is
nowhere documented.
> 1) this option did not broke any exist compatibilities
> 2) there we talk not only about uboot shell, same time will be useful
> to have env_resolve for internal c usage, because env_set dont have
> this feature
I did not say that an "eval" like construct would not be useful.
But uncontrolled recursion with an undocumented depth limit is
a problem.
> yes i'm informed about this plans (and think its happens not so
> soon - but i provide some simple elegant solution already)
> but again we dont have env_resolve for internal c usage which must be
> very useful
On the CLI, we use the "run" command to get the desired effect. Yes,
this is neither perfect nor elegant. But you can use that in C code
as well.
> will be easy get useful features via simple solution ( deep resolve
> all vars by one line )
I understand what you want, but this is not a good way to solve the
problem. I'd really rather see such efforts invested in helping
Francis with the hush update - which will make such code unnecessary.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
Success in marriage is not so much finding the right person as it is
being the right person.
next prev parent reply other threads:[~2021-11-20 12:36 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-19 4:36 [PATCH v2 0/2] env: setenv add resolve value option Artem Lapkin
2021-11-19 4:36 ` [PATCH v2 1/2] " Artem Lapkin
2021-11-25 0:13 ` Simon Glass
2021-11-19 4:36 ` [PATCH v2 2/2] test: env: deep resolve value testing Artem Lapkin
2021-11-25 0:13 ` Simon Glass
2021-11-19 7:48 ` [PATCH v2 0/2] env: setenv add resolve value option Wolfgang Denk
2021-11-19 9:06 ` Art Nikpal
2021-11-20 12:36 ` Wolfgang Denk [this message]
2021-11-22 8:25 ` Art Nikpal
[not found] ` <6198ebca.1c69fb81.fc50c.bf0bSMTPIN_ADDED_BROKEN@mx.google.com>
2021-11-22 8:09 ` Art Nikpal
2021-11-22 8:51 ` Wolfgang Denk
2021-11-22 10:06 ` Art Nikpal
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=2235982.1637411769@gemini.denx.de \
--to=wd@denx.de \
--cc=andre.przywara@arm.com \
--cc=art@khadas.com \
--cc=christianshewitt@gmail.com \
--cc=email2tema@gmail.com \
--cc=francis.laniel@amarulasolutions.com \
--cc=gouwa@khadas.com \
--cc=marek.behun@nic.cz \
--cc=narmstrong@baylibre.com \
--cc=nick@khadas.com \
--cc=sjg@chromium.org \
--cc=trini@konsulko.com \
--cc=twarren@nvidia.com \
--cc=u-boot-amlogic@groups.io \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).