[PATCH v2] Makefile: fix generating environment file
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
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
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
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
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
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
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
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
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
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!
participants (3)
-
Oleksandr Suvorov -
Rasmus Villemoes -
Tom Rini