| Message ID | 1319259100-11376-3-git-send-email-sjg@chromium.org |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Oct 22, 2011 at 00:51, Simon Glass <sjg@chromium.org> wrote: > +int setenv_ulong(const char *varname, ulong value) > +{ > + char *str = simple_itoa(value); > + > + return setenv(varname, str); > +} could be a one liner, but works either way > +int setenv_addr(const char *varname, const void *addr) > +{ > + char str[17]; char str[sizeof(addr) * 2 + 1]; > + sprintf(str, "%x", (uintptr_t)addr); i wonder if we should use %p and drop the cast -mike
Hi Mike On Sat, Oct 22, 2011 at 10:29 PM, Mike Frysinger <vapier@gentoo.org> wrote: > On Sat, Oct 22, 2011 at 00:51, Simon Glass <sjg@chromium.org> wrote: >> +int setenv_ulong(const char *varname, ulong value) >> +{ >> + char *str = simple_itoa(value); >> + >> + return setenv(varname, str); >> +} > > could be a one liner, but works either way OK, was trying to separate that out deliberately. > >> +int setenv_addr(const char *varname, const void *addr) >> +{ >> + char str[17]; > > char str[sizeof(addr) * 2 + 1]; Yes of course! > >> + sprintf(str, "%x", (uintptr_t)addr); > > i wonder if we should use %p and drop the cast > -mike > Is %p supposed to print a 0x before it or not? I saw some discussion about this. I vote for %p no, and %#p yes. I will tidy these up for v3. Regards, Simon
On Tue, Oct 25, 2011 at 17:35, Simon Glass wrote: > On Sat, Oct 22, 2011 at 10:29 PM, Mike Frysinger wrote: >> On Sat, Oct 22, 2011 at 00:51, Simon Glass wrote: >>> + sprintf(str, "%x", (uintptr_t)addr); >> >> i wonder if we should use %p and drop the cast > > Is %p supposed to print a 0x before it or not? I saw some discussion > about this. I vote for %p no, and %#p yes. %p does not currently output a leading 0x. it should (imo), and i'll prob send a patch to fix that. but %x doesn't output a leading 0x either :). -mike
Hi Mike, On Tue, Oct 25, 2011 at 2:39 PM, Mike Frysinger <vapier@gentoo.org> wrote: > On Tue, Oct 25, 2011 at 17:35, Simon Glass wrote: >> On Sat, Oct 22, 2011 at 10:29 PM, Mike Frysinger wrote: >>> On Sat, Oct 22, 2011 at 00:51, Simon Glass wrote: >>>> + sprintf(str, "%x", (uintptr_t)addr); >>> >>> i wonder if we should use %p and drop the cast >> >> Is %p supposed to print a 0x before it or not? I saw some discussion >> about this. I vote for %p no, and %#p yes. > > %p does not currently output a leading 0x. it should (imo), and i'll > prob send a patch to fix that. > > but %x doesn't output a leading 0x either :). > -mike > OK, well to be consistent with glibc we should probably add 0x as you say. isn't it intended that env variables without a leading 0x in the value be interpreted as hex still? Iwc do I really want the 0x? Regards, Simon
Dear Simon Glass, In message <CAPnjgZ1+nTaAjYxq5Nh3i7XTh+RhPC9edVtHF34YNPA2PJcCuw@mail.gmail.com> you wrote: > > isn't it intended that env variables without a leading 0x in the value > be interpreted as hex still? Iwc do I really want the 0x? Yes, this is intended. No, we do NOT want to see the 0x in such cases. Best regards, Wolfgang Denk
On Tue, Oct 25, 2011 at 18:01, Wolfgang Denk wrote: > Simon Glass wrote: >> isn't it intended that env variables without a leading 0x in the value >> be interpreted as hex still? Iwc do I really want the 0x? > > Yes, this is intended. No, we do NOT want to see the 0x in such > cases. ok, so we'll want to stick with %x like you already have Simon. i disagree, but not enough to fight over it ;). -mike
Hi Mike, On Tue, Oct 25, 2011 at 3:08 PM, Mike Frysinger <vapier@gentoo.org> wrote: > On Tue, Oct 25, 2011 at 18:01, Wolfgang Denk wrote: >> Simon Glass wrote: >>> isn't it intended that env variables without a leading 0x in the value >>> be interpreted as hex still? Iwc do I really want the 0x? >> >> Yes, this is intended. No, we do NOT want to see the 0x in such >> cases. > > ok, so we'll want to stick with %x like you already have Simon. i > disagree, but not enough to fight over it ;). > -mike > OK. But I can use %p if you don't send your patch. How about %#p to display 0x? It would be incompatible with glibc, but seems more sensible. Then we could use this in quite a few places in U-Boot I suspect. Regards, Simon
On Tue, Oct 25, 2011 at 18:41, Simon Glass wrote: > On Tue, Oct 25, 2011 at 3:08 PM, Mike Frysinger wrote: >> On Tue, Oct 25, 2011 at 18:01, Wolfgang Denk wrote: >>> Simon Glass wrote: >>>> isn't it intended that env variables without a leading 0x in the value >>>> be interpreted as hex still? Iwc do I really want the 0x? >>> >>> Yes, this is intended. No, we do NOT want to see the 0x in such >>> cases. >> >> ok, so we'll want to stick with %x like you already have Simon. i >> disagree, but not enough to fight over it ;). > > OK. But I can use %p if you don't send your patch. How about %#p to > display 0x? It would be incompatible with glibc, but seems more > sensible. Then we could use this in quite a few places in U-Boot I > suspect. i think wolfgang's point is that he doesn't want "0x" in env vars. i don't think he made any statement about %p in general. also, %#p works the same under u-boot as under glibc. it's just that %#p generates a gcc warning. -mike
Hi Mike, On Tue, Oct 25, 2011 at 3:49 PM, Mike Frysinger <vapier@gentoo.org> wrote: > On Tue, Oct 25, 2011 at 18:41, Simon Glass wrote: >> On Tue, Oct 25, 2011 at 3:08 PM, Mike Frysinger wrote: >>> On Tue, Oct 25, 2011 at 18:01, Wolfgang Denk wrote: >>>> Simon Glass wrote: >>>>> isn't it intended that env variables without a leading 0x in the value >>>>> be interpreted as hex still? Iwc do I really want the 0x? >>>> >>>> Yes, this is intended. No, we do NOT want to see the 0x in such >>>> cases. >>> >>> ok, so we'll want to stick with %x like you already have Simon. i >>> disagree, but not enough to fight over it ;). >> >> OK. But I can use %p if you don't send your patch. How about %#p to >> display 0x? It would be incompatible with glibc, but seems more >> sensible. Then we could use this in quite a few places in U-Boot I >> suspect. > > i think wolfgang's point is that he doesn't want "0x" in env vars. i > don't think he made any statement about %p in general. Yes I got that. > > also, %#p works the same under u-boot as under glibc. it's just that > %#p generates a gcc warning. > -mike > No I meant that for me glibc displays 0x whether or not I put # in there...seems wrong to me. Regards, Simon
On Tue, Oct 25, 2011 at 19:08, Simon Glass wrote: > On Tue, Oct 25, 2011 at 3:49 PM, Mike Frysinger wrote: >> also, %#p works the same under u-boot as under glibc. it's just that >> %#p generates a gcc warning. > > No I meant that for me glibc displays 0x whether or not I put # in > there...seems wrong to me. i don't think so. it's long done this, as does gcc and the Linux kernel. fixing u-boot to operate the same way as the rest of the ecosystem makes sense to me. -mike
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c index 101bc49..da7028c 100644 --- a/common/cmd_nvedit.c +++ b/common/cmd_nvedit.c @@ -377,6 +377,35 @@ int setenv(const char *varname, const char *varvalue) return _do_env_set(0, 3, (char * const *)argv); } +/** + * Set an environment variable to an integer value + * + * @param varname Environmet variable to set + * @param value Value to set it to + * @return 0 if ok, 1 on error + */ +int setenv_ulong(const char *varname, ulong value) +{ + char *str = simple_itoa(value); + + return setenv(varname, str); +} + +/** + * Set an environment variable to an address in hex + * + * @param varname Environmet variable to set + * @param addr Value to set it to + * @return 0 if ok, 1 on error + */ +int setenv_addr(const char *varname, const void *addr) +{ + char str[17]; + + sprintf(str, "%x", (uintptr_t)addr); + return setenv(varname, str); +} + int do_env_set(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { if (argc < 2) diff --git a/include/common.h b/include/common.h index 55f772d..99c2a37 100644 --- a/include/common.h +++ b/include/common.h @@ -294,6 +294,8 @@ int inline setenv (const char *, const char *); #else int setenv (const char *, const char *); #endif /* CONFIG_PPC */ +int setenv_ulong(const char *varname, ulong value); +int setenv_addr(const char *varname, const void *addr); #ifdef CONFIG_ARM # include <asm/mach-types.h> # include <asm/setup.h>
It seems we put numbers and addresses into environment variables a lot. We should have some functions to do this. Signed-off-by: Simon Glass <sjg@chromium.org> --- common/cmd_nvedit.c | 29 +++++++++++++++++++++++++++++ include/common.h | 2 ++ 2 files changed, 31 insertions(+), 0 deletions(-)