[Toybox] pathological case in sed s///g
i've known about this for a couple of days and haven't had time to look at it properly yet, so i should mention it here... if you have a file with a 1MiB line of 'x'es and you sed 's/x/y/g', BSD or GNU sed finishes immediately, but toybox takes forever. ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
[Toybox] [PATCH] env: fix case where a variable is replaced.
Found when trying to update the toybox prebuilt used for the Android
build.
Also add the corresponding test.
---
lib/env.c | 4 +---
tests/env.test | 2 ++
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/env.c b/lib/env.c
index 35ef688c..bc23b753 100644
--- a/lib/env.c
+++ b/lib/env.c
@@ -58,11 +58,9 @@ void xsetenv(char *name, char *val)
if (!memcmp(name, environ[i], len) && environ[i][len]=='=') {
if (i>=envc) free(environ[i]);
else {
-char **delete = environ+i;
-
// move old entries down, add at end of old data
toys.envc = envc--;
-for (i=0; new ? i /dev/null ; echo $?' "125\n" "" ""
testcmd "why is this allowed" "=BLAH env | grep '^=BLAH\$'" "=BLAH\n" "" ""
+
+testcmd "replace" "A=foo PATH= `which printenv` A" "foo\n" "" ""
--
2.21.0.1020.gf2820cf01a-goog
From dbf076d513071b4471a208bf77406d4f404bc70f Mon Sep 17 00:00:00 2001
From: Elliott Hughes
Date: Thu, 2 May 2019 18:41:11 -0700
Subject: [PATCH] env: fix case where a variable is replaced.
Found when trying to update the toybox prebuilt used for the Android
build.
Also add the corresponding test.
---
lib/env.c | 4 +---
tests/env.test | 2 ++
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/env.c b/lib/env.c
index 35ef688c..bc23b753 100644
--- a/lib/env.c
+++ b/lib/env.c
@@ -58,11 +58,9 @@ void xsetenv(char *name, char *val)
if (!memcmp(name, environ[i], len) && environ[i][len]=='=') {
if (i>=envc) free(environ[i]);
else {
-char **delete = environ+i;
-
// move old entries down, add at end of old data
toys.envc = envc--;
-for (i=0; new ? i /dev/null ; echo $?' "125\n" "" ""
testcmd "why is this allowed" "=BLAH env | grep '^=BLAH\$'" "=BLAH\n" "" ""
+
+testcmd "replace" "A=foo PATH= `which printenv` A" "foo\n" "" ""
--
2.21.0.1020.gf2820cf01a-goog
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] FYI: kernel builds rely on GNU expr extensions
(fwiw, with an instrumented build of the 4.19 Android common kernel, i only see `expr 4 * 65536 + 019 * 256 + 037`. so i don't know what they're _really_ doing to hit this.) On Thu, May 2, 2019 at 9:15 AM Rob Landley wrote: > > On 5/1/19 3:23 PM, enh via Toybox wrote: > > so you know how i said i wasn't going to use anything in pending for > > the build? turns out i wasn't paying enough attention and accidentally > > used expr... > > Eh, if you're deploying pending on the device using it in the build isn't that > much more of a lift... > > ALso, $(( )) is expr the same way [ ] is test, so I've gotta finish and > promote > it soon to make toysh work... > > > i'm reverting that now (but keeping it for the device), but folks > > building their kernels from within an Android tree hit this. (we > > recommend that you _don't_ build your kernel that way, and we don't do > > that ourselves, but realistically we're never going to stop everyone > > from doing it. and i know "build the kernel with toybox" is > > interesting to you anyway.) > > It's explicitly one of my goals, to make that supportable, yes. :) > > > so, yeah, the kernel build uses some GNU extensions. `index` looks > > like it's there (though perhaps only for tests?), and the specific bug > > report i got was about GNU unary + which the info page describes thus: > > > > >Strings are not quoted for ‘expr’ itself, though you may need to > > quote them to protect characters with special meaning to the shell, > > e.g., spaces. However, regardless of whether it is quoted, a string > > operand should not be a parenthesis or any of ‘expr’’s operators like > > ‘+’, so you cannot safely pass an arbitrary string ‘$str’ to expr merely > > by quoting it to the shell. One way to work around this is to use the > > GNU extension ‘+’, (e.g., ‘+ "$str" = foo’); a more portable way is to > > use ‘" $str"’ and to adjust the rest of the expression to take the > > leading space into account (e.g., ‘" $str" = " foo"’). > > That was confusing so I looked at the man page: > >+ TOKEN > interpret TOKEN as a string, even if it is a > keyword like 'match' or an operator like '/' > > $ expr + match > match > $ expr 1+2 > 1+2 > $ expr 1 +2 > expr: syntax error > $ expr 1 + 2 > 3 > > Seems straightforward enough. And now I remember why implementing $(( )) with > common plumbing with expr is still on my todo list. :) > > > though i'm honestly not convinced this is their real problem. the > > specific example they give is this from scripts/Makefile.lib in the > > kernel: > > > > size_append = printf $(shell \ > > dec_size=0; \ > > for F in $1; do \ > > fsize=$$($(CONFIG_SHELL) $(srctree)/scripts/file-size.sh $$F); \ > > dec_size=$$(expr $$dec_size + $$fsize); \ > > done; \ > > ... > > > > but given the `dec_size=0` line, i think they're confused and whatever > > failure they're seeing is coming from somewhere else... > > Fun. I'll throw it on the todo pile and shuffle "expr" near the top of the > promotion heap... > > Thanks, > > Rob > ___ > Toybox mailing list > Toybox@lists.landley.net > http://lists.landley.net/listinfo.cgi/toybox-landley.net ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] FYI: kernel builds rely on GNU expr extensions
On Thu, May 2, 2019 at 9:15 AM Rob Landley wrote: > > On 5/1/19 3:23 PM, enh via Toybox wrote: > > so you know how i said i wasn't going to use anything in pending for > > the build? turns out i wasn't paying enough attention and accidentally > > used expr... > > Eh, if you're deploying pending on the device using it in the build isn't that > much more of a lift... i know what you mean, and you've probably heard my explanation before, but... on the device, the choice is "something or nothing". "something" almost always wins. you have to be really bad to be worse than nothing :-) (the one curious example on the device was dd where we did have pretty decent BSD dd but enough people wanted the ability to use hex in arguments to make it worth switching early.) on the host, the choice is "toybox or GNU". by definition, you already have something that works, so it's a tougher sell. > ALso, $(( )) is expr the same way [ ] is test, so I've gotta finish and > promote > it soon to make toysh work... > > > i'm reverting that now (but keeping it for the device), but folks > > building their kernels from within an Android tree hit this. (we > > recommend that you _don't_ build your kernel that way, and we don't do > > that ourselves, but realistically we're never going to stop everyone > > from doing it. and i know "build the kernel with toybox" is > > interesting to you anyway.) > > It's explicitly one of my goals, to make that supportable, yes. :) > > > so, yeah, the kernel build uses some GNU extensions. `index` looks > > like it's there (though perhaps only for tests?), and the specific bug > > report i got was about GNU unary + which the info page describes thus: > > > > >Strings are not quoted for ‘expr’ itself, though you may need to > > quote them to protect characters with special meaning to the shell, > > e.g., spaces. However, regardless of whether it is quoted, a string > > operand should not be a parenthesis or any of ‘expr’’s operators like > > ‘+’, so you cannot safely pass an arbitrary string ‘$str’ to expr merely > > by quoting it to the shell. One way to work around this is to use the > > GNU extension ‘+’, (e.g., ‘+ "$str" = foo’); a more portable way is to > > use ‘" $str"’ and to adjust the rest of the expression to take the > > leading space into account (e.g., ‘" $str" = " foo"’). > > That was confusing so I looked at the man page: > >+ TOKEN > interpret TOKEN as a string, even if it is a > keyword like 'match' or an operator like '/' > > $ expr + match > match > $ expr 1+2 > 1+2 > $ expr 1 +2 > expr: syntax error > $ expr 1 + 2 > 3 > > Seems straightforward enough. And now I remember why implementing $(( )) with > common plumbing with expr is still on my todo list. :) > > > though i'm honestly not convinced this is their real problem. the > > specific example they give is this from scripts/Makefile.lib in the > > kernel: > > > > size_append = printf $(shell \ > > dec_size=0; \ > > for F in $1; do \ > > fsize=$$($(CONFIG_SHELL) $(srctree)/scripts/file-size.sh $$F); \ > > dec_size=$$(expr $$dec_size + $$fsize); \ > > done; \ > > ... > > > > but given the `dec_size=0` line, i think they're confused and whatever > > failure they're seeing is coming from somewhere else... > > Fun. I'll throw it on the todo pile and shuffle "expr" near the top of the > promotion heap... > > Thanks, > > Rob > ___ > Toybox mailing list > Toybox@lists.landley.net > http://lists.landley.net/listinfo.cgi/toybox-landley.net ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] [PATCH] Android moved the scheduler policy functions in Q.
On Thu, May 2, 2019 at 9:35 AM Rob Landley wrote: > > On 5/1/19 5:40 PM, enh via Toybox wrote: > > They're forwarded to libprocessgroup, but we may as well go straight to > > the source since neither library is in the NDK anyway. > > > > This code is unfortunate because it means that even `toybox true` ends > > up pulling in a JSON parser at runtime, because ps might call > > get_sched_policy/get_sched_policy_name. I'll experiment with > > dlopen-on-demand in portability.c and see whether the savings are > > worthwhile, but for now at least use the current library directly so we > > can save *one* dlopen! > > I'm as-needed in the linker but glibc defeated that by having librt pull in > libpthread when clock_gettime() has been part of glibc proper since the 2.17 > release ~7 years ago. does as-needed actually help when you're built RELRO? certainly Android does everything up front for RELRO (which has been mandatory for years). i don't know what glibc does, but i do know that my linux boxes are at least using partial RELRO these days. > Meanwhile, Android seem to have reinvented the microkernel again and turned > everything in the world into message passing: > > https://security.googleblog.com/2019/03/open-sourcing-sandboxed-api.html (fwiw, i think that's Chrome. certainly nothing to do with us.) > But then as far as I could tell flatpak and friends mostly exist because > people > forgot about static linking. (A shared library to fake a mount point from a > statically linked zipfile could be fun, and sounds less complicated than > fakeroot or crunchgen were, but no: we have this new container infrastructure > so > everything looks like a nail...) > > https://www.youtube.com/watch?v=ePcDSutt_Ww > > Anyway, applied. > > Rob ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] [PATCH] tar.test: workaround for mksh.
On 5/1/19 2:36 PM, enh wrote: > thanks. thanks for improving my echo -E patch too --- i knew about the > syntax for "error if given more than one of these options", but not > "take the last of this set to appear". Options switching other options off is pretty common and needed a thing. (Another reason I tend to want a short option for each --long-opt is the square bracket grouping syntax only applies to short options. Works fine for a --longopt that _has_ a corresponding short option, but I never came up with a syntax to let you put a longopt in a group.) Rob ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] [PATCH] Android moved the scheduler policy functions in Q.
On 5/1/19 5:40 PM, enh via Toybox wrote: > They're forwarded to libprocessgroup, but we may as well go straight to > the source since neither library is in the NDK anyway. > > This code is unfortunate because it means that even `toybox true` ends > up pulling in a JSON parser at runtime, because ps might call > get_sched_policy/get_sched_policy_name. I'll experiment with > dlopen-on-demand in portability.c and see whether the savings are > worthwhile, but for now at least use the current library directly so we > can save *one* dlopen! I'm as-needed in the linker but glibc defeated that by having librt pull in libpthread when clock_gettime() has been part of glibc proper since the 2.17 release ~7 years ago. Meanwhile, Android seem to have reinvented the microkernel again and turned everything in the world into message passing: https://security.googleblog.com/2019/03/open-sourcing-sandboxed-api.html But then as far as I could tell flatpak and friends mostly exist because people forgot about static linking. (A shared library to fake a mount point from a statically linked zipfile could be fun, and sounds less complicated than fakeroot or crunchgen were, but no: we have this new container infrastructure so everything looks like a nail...) https://www.youtube.com/watch?v=ePcDSutt_Ww Anyway, applied. Rob ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] FYI: kernel builds rely on GNU expr extensions
On 5/1/19 3:23 PM, enh via Toybox wrote: > so you know how i said i wasn't going to use anything in pending for > the build? turns out i wasn't paying enough attention and accidentally > used expr... Eh, if you're deploying pending on the device using it in the build isn't that much more of a lift... ALso, $(( )) is expr the same way [ ] is test, so I've gotta finish and promote it soon to make toysh work... > i'm reverting that now (but keeping it for the device), but folks > building their kernels from within an Android tree hit this. (we > recommend that you _don't_ build your kernel that way, and we don't do > that ourselves, but realistically we're never going to stop everyone > from doing it. and i know "build the kernel with toybox" is > interesting to you anyway.) It's explicitly one of my goals, to make that supportable, yes. :) > so, yeah, the kernel build uses some GNU extensions. `index` looks > like it's there (though perhaps only for tests?), and the specific bug > report i got was about GNU unary + which the info page describes thus: >Strings are not quoted for ‘expr’ itself, though you may need to > quote them to protect characters with special meaning to the shell, > e.g., spaces. However, regardless of whether it is quoted, a string > operand should not be a parenthesis or any of ‘expr’’s operators like > ‘+’, so you cannot safely pass an arbitrary string ‘$str’ to expr merely > by quoting it to the shell. One way to work around this is to use the > GNU extension ‘+’, (e.g., ‘+ "$str" = foo’); a more portable way is to > use ‘" $str"’ and to adjust the rest of the expression to take the > leading space into account (e.g., ‘" $str" = " foo"’). That was confusing so I looked at the man page: + TOKEN interpret TOKEN as a string, even if it is a keyword like 'match' or an operator like '/' $ expr + match match $ expr 1+2 1+2 $ expr 1 +2 expr: syntax error $ expr 1 + 2 3 Seems straightforward enough. And now I remember why implementing $(( )) with common plumbing with expr is still on my todo list. :) > though i'm honestly not convinced this is their real problem. the > specific example they give is this from scripts/Makefile.lib in the > kernel: > > size_append = printf $(shell \ > dec_size=0; \ > for F in $1; do \ > fsize=$$($(CONFIG_SHELL) $(srctree)/scripts/file-size.sh $$F); \ > dec_size=$$(expr $$dec_size + $$fsize); \ > done; \ > ... > > but given the `dec_size=0` line, i think they're confused and whatever > failure they're seeing is coming from somewhere else... Fun. I'll throw it on the todo pile and shuffle "expr" near the top of the promotion heap... Thanks, Rob ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
