* [PATCH v2] Makefile: fix generating environment file @ 2021-04-20 14:43 Oleksandr Suvorov 2021-04-20 19:33 ` Rasmus Villemoes 0 siblings, 1 reply; 10+ messages in thread From: Oleksandr Suvorov @ 2021-04-20 14:43 UTC (permalink / raw) To: u-boot If the CONFIG_USE_DEFAULT_ENV_FILE=y and CONFIG_DEFAULT_ENV_FILE points to the empty environment file, the auto-generated file has the wrong syntax so it leads to the compilation failure: In file included from include/env_default.h:115, from env/common.c:29: include/generated/defaultenv_autogenerated.h:1:1: error: expected expression before ?,? token 1 | , 0x00 | ^ make[1]: *** [scripts/Makefile.build:266: env/common.o] Error 1 make[1]: *** Waiting for unfinished jobs.... make: *** [Makefile:1744: env] Error 2 Fix this issue conditionally adding the delimiter ", ". Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com> --- Changes in v2: - Fix the hex-decimal class matching. Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index e423f6de746..4e0a9b51cb7 100644 --- a/Makefile +++ b/Makefile @@ -1853,7 +1853,9 @@ define filechk_defaultenv.h grep -v '^$$' | \ tr '\n' '\0' | \ sed -e 's/\\\x0\s*//g' | \ - xxd -i ; echo ", 0x00" ; ) + xxd -i | \ + sed -r 's/([0-9a-f])$$/\1, /'; \ + echo "0x00" ; ) endef define filechk_dt.h -- 2.31.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2] Makefile: fix generating environment file 2021-04-20 14:43 [PATCH v2] Makefile: fix generating environment file Oleksandr Suvorov @ 2021-04-20 19:33 ` Rasmus Villemoes 2021-04-20 21:10 ` Oleksandr Suvorov 0 siblings, 1 reply; 10+ messages in thread From: Rasmus Villemoes @ 2021-04-20 19:33 UTC (permalink / raw) To: u-boot On 20/04/2021 16.43, Oleksandr Suvorov wrote: > If the CONFIG_USE_DEFAULT_ENV_FILE=y and CONFIG_DEFAULT_ENV_FILE > points to the empty environment file, the auto-generated file has > the wrong syntax so it leads to the compilation failure: > Glad someone is using CONFIG_USE_DEFAULT_ENV_FILE :) And thanks for reporting this. > > Fix this issue conditionally adding the delimiter ", ". Hm, yeah, that should work. But I wonder if it would make more sense to ensure tr always gets a final newline (which then gets translated to a nul byte, which in turn gives the trailing 0x00). Something like (untested) define filechk_defaultenv.h ( { grep -v '^#' | grep -v '^$$' ; echo '' ; } | \ tr '\n' '\0' | \ sed -e 's/\\\x0\s*//g' | \ xxd -i ; ) endef Rasmus ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] Makefile: fix generating environment file 2021-04-20 19:33 ` Rasmus Villemoes @ 2021-04-20 21:10 ` Oleksandr Suvorov 2021-04-20 21:33 ` Rasmus Villemoes 0 siblings, 1 reply; 10+ messages in thread From: Oleksandr Suvorov @ 2021-04-20 21:10 UTC (permalink / raw) To: u-boot Hi Rasmus, Thanks for your feedback! Yes, I noted that there were no possible situations with the trailing code != 0x00, but simply removing the additional trailing 0x00 gives us an empty array default_environment[] for the empty defaultenv file. I need to test whether this case is handled in u-boot properly and then prepare the next patch version :P On Tue, Apr 20, 2021 at 10:33 PM Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote: > > On 20/04/2021 16.43, Oleksandr Suvorov wrote: > > If the CONFIG_USE_DEFAULT_ENV_FILE=y and CONFIG_DEFAULT_ENV_FILE > > points to the empty environment file, the auto-generated file has > > the wrong syntax so it leads to the compilation failure: > > > > Glad someone is using CONFIG_USE_DEFAULT_ENV_FILE :) And thanks for > reporting this. > > > > > Fix this issue conditionally adding the delimiter ", ". > > Hm, yeah, that should work. But I wonder if it would make more sense to > ensure tr always gets a final newline (which then gets translated to a > nul byte, which in turn gives the trailing 0x00). Something like (untested) > > define filechk_defaultenv.h > ( { grep -v '^#' | grep -v '^$$' ; echo '' ; } | \ > tr '\n' '\0' | \ > sed -e 's/\\\x0\s*//g' | \ > xxd -i ; ) > endef > > Rasmus -- Best regards Oleksandr Suvorov Toradex AG Ebenaustrasse 10 | 6048 Horw | Switzerland | T: +41 41 500 48 00 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] Makefile: fix generating environment file 2021-04-20 21:10 ` Oleksandr Suvorov @ 2021-04-20 21:33 ` Rasmus Villemoes 2021-04-21 15:21 ` Oleksandr Suvorov 0 siblings, 1 reply; 10+ messages in thread From: Rasmus Villemoes @ 2021-04-20 21:33 UTC (permalink / raw) To: u-boot On 20/04/2021 23.10, Oleksandr Suvorov wrote: > Hi Rasmus, > > Thanks for your feedback! > Yes, I noted that there were no possible situations with the trailing > code != 0x00, but simply removing the additional trailing 0x00 > gives us an empty array default_environment[] for the empty defaultenv file. > I need to test whether this case is handled in u-boot properly and > then prepare the next patch version :P No, I'm not suggesting removing the trailing nul byte, it very much has to be there - the binary format of the environment is a sequence of nul-terminated C strings of the key=value form, concatenated back-to-back, terminated by an empty string. What I'm suggesting is to take the input file === foo=bar # Set our IP address ip=1.2.3.4 === do the comment- and empty-line stripping (the two first greps), and then after that add an extra empty line === foo=bar ip=1.2.3.4 === and then feed that to the 'replace \n by nul bytes' | 'delete backslash+nul+whitespace' | xxd pipe. That way there's always that trailing nul on the input to xxd, i.e. in the example above, we would feed foo=bar\0ip-1.2.3.4\0\0 into xxd, while with an initially empty file xxd would just receive that single nul byte. It's just that I think terminating the sequence of key=value lines by an empty line more exactly matches the binary format. Rasmus ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] Makefile: fix generating environment file 2021-04-20 21:33 ` Rasmus Villemoes @ 2021-04-21 15:21 ` Oleksandr Suvorov 2021-04-21 20:55 ` Rasmus Villemoes 0 siblings, 1 reply; 10+ messages in thread From: Oleksandr Suvorov @ 2021-04-21 15:21 UTC (permalink / raw) To: u-boot Hi Rasmus, On Wed, Apr 21, 2021 at 12:34 AM Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote: > > On 20/04/2021 23.10, Oleksandr Suvorov wrote: > > Hi Rasmus, > > > > Thanks for your feedback! > > Yes, I noted that there were no possible situations with the trailing > > code != 0x00, but simply removing the additional trailing 0x00 > > gives us an empty array default_environment[] for the empty defaultenv file. > > I need to test whether this case is handled in u-boot properly and > > then prepare the next patch version :P > > No, I'm not suggesting removing the trailing nul byte, it very much has > to be there - the binary format of the environment is a sequence of > nul-terminated C strings of the key=value form, concatenated > back-to-back, terminated by an empty string. (/me saying: never answer at night, never answer at night, never answer at night :-D) > > What I'm suggesting is to take the input file > > === > foo=bar > > # Set our IP address > ip=1.2.3.4 > === > > do the comment- and empty-line stripping (the two first greps), and then > after that add an extra empty line > > === > foo=bar > ip=1.2.3.4 > > === > > and then feed that to the 'replace \n by nul bytes' | 'delete > backslash+nul+whitespace' | xxd pipe. That way there's always that > trailing nul on the input to xxd, i.e. in the example above, we would > feed foo=bar\0ip-1.2.3.4\0\0 into xxd, while with an initially empty > file xxd would just receive that single nul byte. > > It's just that I think terminating the sequence of key=value lines by an > empty line more exactly matches the binary format. Sure, now I see. Your solution is more straight and clear. Unfortunately, it doesn't work :) So if you don't mind, I'll try to make it work as it should and post the next version of the patch. > > Rasmus -- Best regards Oleksandr Suvorov Toradex AG Ebenaustrasse 10 | 6048 Horw | Switzerland | T: +41 41 500 48 00 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] Makefile: fix generating environment file 2021-04-21 15:21 ` Oleksandr Suvorov @ 2021-04-21 20:55 ` Rasmus Villemoes 2021-04-22 6:34 ` Oleksandr Suvorov 0 siblings, 1 reply; 10+ messages in thread From: Rasmus Villemoes @ 2021-04-21 20:55 UTC (permalink / raw) To: u-boot On 21/04/2021 17.21, Oleksandr Suvorov wrote: > Hi Rasmus, > > On Wed, Apr 21, 2021 at 12:34 AM Rasmus Villemoes > <rasmus.villemoes@prevas.dk> wrote: >> >> On 20/04/2021 23.10, Oleksandr Suvorov wrote: >>> Hi Rasmus, >>> >>> Thanks for your feedback! >>> Yes, I noted that there were no possible situations with the trailing >>> code != 0x00, but simply removing the additional trailing 0x00 >>> gives us an empty array default_environment[] for the empty defaultenv file. >>> I need to test whether this case is handled in u-boot properly and >>> then prepare the next patch version :P >> >> No, I'm not suggesting removing the trailing nul byte, it very much has >> to be there - the binary format of the environment is a sequence of >> nul-terminated C strings of the key=value form, concatenated >> back-to-back, terminated by an empty string. > > (/me saying: never answer at night, never answer at night, never > answer at night :-D) > >> >> What I'm suggesting is to take the input file >> >> === >> foo=bar >> >> # Set our IP address >> ip=1.2.3.4 >> === >> >> do the comment- and empty-line stripping (the two first greps), and then >> after that add an extra empty line >> >> === >> foo=bar >> ip=1.2.3.4 >> >> === >> >> and then feed that to the 'replace \n by nul bytes' | 'delete >> backslash+nul+whitespace' | xxd pipe. That way there's always that >> trailing nul on the input to xxd, i.e. in the example above, we would >> feed foo=bar\0ip-1.2.3.4\0\0 into xxd, while with an initially empty >> file xxd would just receive that single nul byte. >> >> It's just that I think terminating the sequence of key=value lines by an >> empty line more exactly matches the binary format. > > Sure, now I see. Your solution is more straight and clear. > Unfortunately, it doesn't work :) Yeah, I didn't really expect it to. Ah, it's because "set -e" is in effect, so in ( { grep -v '^#' | grep -v '^$$' ; echo '' ; } | \ the return value of the grep -v '^#' | grep -v '^$$' pipeline is that of the second grep, and when there's no input lines that match (such as, with an empty input file), that's an EXIT_FAILURE. So the whole subshell exits at that point, and nothing gets written to defaultenv_autogenerated.h. Doing define filechk_defaultenv.h ( { grep -v '^#' | grep -v '^$$' || true ; echo '' ; } | \ tr '\n' '\0' | \ sed -e 's/\\\x0\s*//g' | \ xxd -i ; ) endef seems to work. Rasmus ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] Makefile: fix generating environment file 2021-04-21 20:55 ` Rasmus Villemoes @ 2021-04-22 6:34 ` Oleksandr Suvorov 2021-04-22 7:44 ` [PATCH] Makefile: fix generation of defaultenv.h from empty initial file Rasmus Villemoes 0 siblings, 1 reply; 10+ messages in thread From: Oleksandr Suvorov @ 2021-04-22 6:34 UTC (permalink / raw) To: u-boot On Wed, Apr 21, 2021 at 11:56 PM Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote: > > On 21/04/2021 17.21, Oleksandr Suvorov wrote: > > Hi Rasmus, > > > > On Wed, Apr 21, 2021 at 12:34 AM Rasmus Villemoes > > <rasmus.villemoes@prevas.dk> wrote: > >> > >> On 20/04/2021 23.10, Oleksandr Suvorov wrote: > >>> Hi Rasmus, > >>> > >>> Thanks for your feedback! > >>> Yes, I noted that there were no possible situations with the trailing > >>> code != 0x00, but simply removing the additional trailing 0x00 > >>> gives us an empty array default_environment[] for the empty defaultenv file. > >>> I need to test whether this case is handled in u-boot properly and > >>> then prepare the next patch version :P > >> > >> No, I'm not suggesting removing the trailing nul byte, it very much has > >> to be there - the binary format of the environment is a sequence of > >> nul-terminated C strings of the key=value form, concatenated > >> back-to-back, terminated by an empty string. > > > > (/me saying: never answer at night, never answer at night, never > > answer at night :-D) > > > >> > >> What I'm suggesting is to take the input file > >> > >> === > >> foo=bar > >> > >> # Set our IP address > >> ip=1.2.3.4 > >> === > >> > >> do the comment- and empty-line stripping (the two first greps), and then > >> after that add an extra empty line > >> > >> === > >> foo=bar > >> ip=1.2.3.4 > >> > >> === > >> > >> and then feed that to the 'replace \n by nul bytes' | 'delete > >> backslash+nul+whitespace' | xxd pipe. That way there's always that > >> trailing nul on the input to xxd, i.e. in the example above, we would > >> feed foo=bar\0ip-1.2.3.4\0\0 into xxd, while with an initially empty > >> file xxd would just receive that single nul byte. > >> > >> It's just that I think terminating the sequence of key=value lines by an > >> empty line more exactly matches the binary format. > > > > Sure, now I see. Your solution is more straight and clear. > > Unfortunately, it doesn't work :) > > Yeah, I didn't really expect it to. Ah, it's because "set -e" is in > effect, so in > > ( { grep -v '^#' | grep -v '^$$' ; echo '' ; } | \ > > the return value of the grep -v '^#' | grep -v '^$$' pipeline is that > of the second grep, and when there's no input lines that match (such as, > with an empty input file), that's an EXIT_FAILURE. So the whole subshell > exits at that point, and nothing gets written to defaultenv_autogenerated.h. > > Doing > > define filechk_defaultenv.h > ( { grep -v '^#' | grep -v '^$$' || true ; echo '' ; } | \ > tr '\n' '\0' | \ > sed -e 's/\\\x0\s*//g' | \ > xxd -i ; ) > endef > > seems to work. So will you post your own patch? > Rasmus -- Best regards Oleksandr Suvorov Toradex AG Ebenaustrasse 10 | 6048 Horw | Switzerland | T: +41 41 500 48 00 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] Makefile: fix generation of defaultenv.h from empty initial file 2021-04-22 6:34 ` Oleksandr Suvorov @ 2021-04-22 7:44 ` Rasmus Villemoes 2021-04-22 9:26 ` Oleksandr Suvorov 2021-04-27 16:46 ` Tom Rini 0 siblings, 2 replies; 10+ messages in thread From: Rasmus Villemoes @ 2021-04-22 7:44 UTC (permalink / raw) To: u-boot When CONFIG_USE_DEFAULT_ENV_FILE=y and the file CONFIG_DEFAULT_ENV_FILE is empty (or at least doesn't contain any non-comment, non-empty lines), we end up feeding nothing into xxd, which in turn then outputs nothing. Then blindly appending ", 0x00" means that we end up trying to compile (roughly) const char defaultenv[] = { , 0x00 } which is of course broken. To fix that, change the frobbing of the text file so that we always end up printing an extra empty line (which gets turned into that extra nul byte we need) - that corresponds better to the binary format consisting of a series of key=val nul terminated strings, terminated by an empty string. Reported-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk> --- Makefile | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 3fc9777b0b..b7af2b936d 100644 --- a/Makefile +++ b/Makefile @@ -1854,11 +1854,10 @@ define filechk_timestamp.h endef define filechk_defaultenv.h - (grep -v '^#' | \ - grep -v '^$$' | \ + ( { grep -v '^#' | grep -v '^$$' || true ; echo '' ; } | \ tr '\n' '\0' | \ sed -e 's/\\\x0\s*//g' | \ - xxd -i ; echo ", 0x00" ; ) + xxd -i ; ) endef define filechk_dt.h -- 2.29.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] Makefile: fix generation of defaultenv.h from empty initial file 2021-04-22 7:44 ` [PATCH] Makefile: fix generation of defaultenv.h from empty initial file Rasmus Villemoes @ 2021-04-22 9:26 ` Oleksandr Suvorov 2021-04-27 16:46 ` Tom Rini 1 sibling, 0 replies; 10+ messages in thread From: Oleksandr Suvorov @ 2021-04-22 9:26 UTC (permalink / raw) To: u-boot Hi Rasmus, Thanks for the patch, I've tested it. On Thu, Apr 22, 2021 at 10:44 AM Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote: > > When CONFIG_USE_DEFAULT_ENV_FILE=y and the file > CONFIG_DEFAULT_ENV_FILE is empty (or at least doesn't contain any > non-comment, non-empty lines), we end up feeding nothing into xxd, > which in turn then outputs nothing. Then blindly appending ", 0x00" > means that we end up trying to compile (roughly) > > const char defaultenv[] = { , 0x00 } > > which is of course broken. > > To fix that, change the frobbing of the text file so that we always > end up printing an extra empty line (which gets turned into that extra > nul byte we need) - that corresponds better to the binary format > consisting of a series of key=val nul terminated strings, terminated > by an empty string. > > Reported-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com> > Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk> Reviewed-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com> > --- > Makefile | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/Makefile b/Makefile > index 3fc9777b0b..b7af2b936d 100644 > --- a/Makefile > +++ b/Makefile > @@ -1854,11 +1854,10 @@ define filechk_timestamp.h > endef > > define filechk_defaultenv.h > - (grep -v '^#' | \ > - grep -v '^$$' | \ > + ( { grep -v '^#' | grep -v '^$$' || true ; echo '' ; } | \ > tr '\n' '\0' | \ > sed -e 's/\\\x0\s*//g' | \ > - xxd -i ; echo ", 0x00" ; ) > + xxd -i ; ) > endef > > define filechk_dt.h > -- > 2.29.2 > -- Best regards Oleksandr Suvorov Toradex AG Ebenaustrasse 10 | 6048 Horw | Switzerland | T: +41 41 500 48 00 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] Makefile: fix generation of defaultenv.h from empty initial file 2021-04-22 7:44 ` [PATCH] Makefile: fix generation of defaultenv.h from empty initial file Rasmus Villemoes 2021-04-22 9:26 ` Oleksandr Suvorov @ 2021-04-27 16:46 ` Tom Rini 1 sibling, 0 replies; 10+ messages in thread From: Tom Rini @ 2021-04-27 16:46 UTC (permalink / raw) To: u-boot On Thu, Apr 22, 2021 at 09:44:18AM +0200, Rasmus Villemoes wrote: > When CONFIG_USE_DEFAULT_ENV_FILE=y and the file > CONFIG_DEFAULT_ENV_FILE is empty (or at least doesn't contain any > non-comment, non-empty lines), we end up feeding nothing into xxd, > which in turn then outputs nothing. Then blindly appending ", 0x00" > means that we end up trying to compile (roughly) > > const char defaultenv[] = { , 0x00 } > > which is of course broken. > > To fix that, change the frobbing of the text file so that we always > end up printing an extra empty line (which gets turned into that extra > nul byte we need) - that corresponds better to the binary format > consisting of a series of key=val nul terminated strings, terminated > by an empty string. > > Reported-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com> > Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk> > Reviewed-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com> Applied to u-boot/master, thanks! -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 659 bytes Desc: not available URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210427/6a9cafe4/attachment.sig> ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-04-27 16:46 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-20 14:43 [PATCH v2] Makefile: fix generating environment file Oleksandr Suvorov 2021-04-20 19:33 ` Rasmus Villemoes 2021-04-20 21:10 ` Oleksandr Suvorov 2021-04-20 21:33 ` Rasmus Villemoes 2021-04-21 15:21 ` Oleksandr Suvorov 2021-04-21 20:55 ` Rasmus Villemoes 2021-04-22 6:34 ` Oleksandr Suvorov 2021-04-22 7:44 ` [PATCH] Makefile: fix generation of defaultenv.h from empty initial file Rasmus Villemoes 2021-04-22 9:26 ` Oleksandr Suvorov 2021-04-27 16:46 ` Tom Rini
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.