RE: Failing shell code under busybox 1.36.1 that worked with 1.31.1
Well, I also tried adding support for bash's ${var@Q} (which I think is the
most readable way to solve quoting) but it seems I really don't understand
how ash's subevalvar() works after all :)
./busybox ash -xvc 'v="ab cd"; echo "(${v@Q})"'
+ v='ab cd'
ash: vstransform:str='Q'
ash: vstransform:startp='ab cd'
+ echo ''"'"'a'"'"'ab cd'"'"')'
'a'ab cd')
diff --git a/shell/ash.c b/shell/ash.c
index ae1b79f37..8d5d6146d 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -222,6 +222,8 @@
#define IF_BASH_PATTERN_SUBST IF_ASH_BASH_COMPAT
#defineBASH_SUBSTR ENABLE_ASH_BASH_COMPAT
#define IF_BASH_SUBSTR IF_ASH_BASH_COMPAT
+#defineBASH_PARAM_TRANSFORM ENABLE_ASH_BASH_COMPAT
+#define IF_BASH_PARAM_TRANSFORM IF_ASH_BASH_COMPAT
/* BASH_TEST2: [[ EXPR ]]
* Status of [[ support:
* && and || work as they should
@@ -574,6 +576,7 @@ var_end(const char *var)
return var;
}
+static int hasmeta(const char *);
/* Parser data */
@@ -868,6 +871,9 @@ out2str(const char *p)
#define VSREPLACE 0xd /* ${var/pattern/replacement} */
#define VSREPLACEALL0xe /* ${var//pattern/replacement} */
#endif
+#if BASH_PARAM_TRANSFORM
+#define VSTRANSFORM 0xf /* ${var@operator} */
+#endif
static const char dolatstr[] ALIGN1 = {
CTLQUOTEMARK, CTLVAR, VSNORMAL, '@', '=', CTLQUOTEMARK, '\0'
@@ -7262,6 +7268,20 @@ subevalvar(char *start, char *str, int strloc,
repl ? NULL : (slash_pos < 0 ? NULL : &slash_pos)
);
+#if BASH_PARAM_TRANSFORM
+ if (subtype == VSTRANSFORM) {
+ bb_error_msg("vstransform:str='%s'", str);
+ if (strcmp(str, "Q") != 0)
+ goto out1;
+ bb_error_msg("vstransform:startp='%s'", startp);
+ char *quoted = single_quote(startp);
+ int len = strlen(quoted);
+ memmove(startp, quoted, len);
+ loc = startp + len;
+ goto out;
+ }
+#endif /* BASH_PARAM_TRANSFORM */
+
#if BASH_PATTERN_SUBST
workloc = expdest - (char *)stackblock();
if (subtype == VSREPLACE || subtype == VSREPLACEALL) {
@@ -7432,7 +7452,7 @@ subevalvar(char *start, char *str, int strloc,
out:
amount = loc - expdest;
STADJUST(amount, expdest);
-#if BASH_PATTERN_SUBST
+#if BASH_PATTERN_SUBST || BASH_PARAM_TRANSFORM
out1:
#endif
/* Remove any recorded regions beyond start of variable */
@@ -7671,6 +7691,9 @@ evalvar(char *p, int flag)
#if BASH_PATTERN_SUBST
case VSREPLACE:
case VSREPLACEALL:
+#endif
+#if BASH_PARAM_TRANSFORM
+ case VSTRANSFORM:
#endif
break;
default:
@@ -12989,6 +13012,13 @@ parsesub: {
goto badsub;
subtype++; /* VSREPLACEALL */
break;
+#endif
+#if BASH_PARAM_TRANSFORM
+ case '@':
+ /* ${v@operator} */
+ subtype = VSTRANSFORM;
+ newsyn = BASESYNTAX;
+ break;
#endif
}
} else {
On Thu, 15 Feb 2024, David Laight wrote:
From: David Leonard
Sent: 15 February 2024 08:46
To: Harvey
Cc: busybox@busybox.net
Subject: Re: Failing shell code under busybox 1.36.1 that worked with 1.31.1
_result="$_result '${_p//'/'\"'\"'}'"
Busybox ash has a CONFIG_ASH_BASH_COMPAT config to get this ${var//...}
replacement behaviour. And, in bash, the ' in the pattern section is treated
as a quote start. To give the ' a literal meaning, use \'
eg ${_p//\'/..repl..}
...
It is probably safer to the quotes into variables.
Then the syntax analysis won't get messed up.
So something like:
sq="'"
qsq='"'"'"'"'
then
"${_p/$sg/$qsq}"
is probably ok and maybe more readable.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: Failing shell code under busybox 1.36.1 that worked with 1.31.1
David,
I can confirm that your patch solves my problem without any further need
to change the shell code. Now V 1.36.1 acts in the same way as V 1.31.1
did. Thank you so mutch! Maybe this is candidate to merge ;)
Greetings
Harvey
Am 15.02.24 um 09:45 schrieb David Leonard:
_result="$_result '${_p//'/'\"'\"'}'"
Busybox ash has a CONFIG_ASH_BASH_COMPAT config to get this ${var//...}
replacement behaviour. And, in bash, the ' in the pattern section is
treated
as a quote start. To give the ' a literal meaning, use \'
eg ${_p//\'/..repl..}
In current busybox ash you will run into a bug where quotes aren't
handled in the pattern, and even loses the rest of the word, as you
observed. That is, given this script:
var="don't g/o"
echo "${var/'/'o}!"
echo "${var/\'/\'o}!"
bash outputs:
don't g! (pattern="/o" repl="")
don'ot g/o! (pattern="'", repl="'o")
and busybox ash outputs:
don't g/o (pattern=?, repl=?, NOTE: lost !)
don'ot g/o! (pattern="'", repl="'o")
Here's a patch to make ash a little bit closer to bash.
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -7058,6 +7058,7 @@ subevalvar(char *start, char *str, int strloc,
*/
repl = NULL;
if (subtype == VSREPLACE || subtype == VSREPLACEALL) {
+ int quoted = 0;
/* Find '/' and replace with NUL */
repl = start;
/* The pattern can't be empty.
@@ -7073,6 +7074,11 @@ subevalvar(char *start, char *str, int strloc,
repl = NULL;
break;
}
+ /* Handle quoted patterns */
+ if ((unsigned char)*repl == CTLQUOTEMARK)
+ quoted ^= 1;
+ if (quoted)
+ goto literal;
if (*repl == '/') {
*repl = '\0';
break;
@@ -7084,6 +7090,7 @@ subevalvar(char *start, char *str, int strloc,
/* Handle escaped slashes, e.g. "${v/\//_}"
(they are CTLESC'ed by this point) */
if ((unsigned char)*repl == CTLESC && repl[1])
repl++;
+ literal:
repl++;
}
}
new file mode 100644
index 0..23aecb5ae
--- /dev/null
+++ b/shell/ash_test/ash-vars/var_bash_repl_quoted.right
@@ -0,0 +1,3 @@
+ab.
+axb.
+x/b.
new file mode 100644
index 0..d2ba39578
--- /dev/null
+++ b/shell/ash_test/ash-vars/var_bash_repl_quoted.tests
@@ -0,0 +1,4 @@
+v=a/b
+echo "${v/'/'}."
+echo "${v/'/'/x}."
+echo "${v/'a'/x}."
On Wed, 14 Feb 2024, Harvey wrote:
Sorry, posted the wrong version of the pack_args() funtion:
I have payed with possible solutions for so long that I forgot the
restore the original before posting -blush-
Here is the original version:
# Packs arguments into a single string.
#
# Input:
# $1 = name of variable receiving the arguments in such a way that
# (re)evaluating them provides the original arguments
# $2... = arguments
# Example:
# func() {
# local args
# pack_args args "$@"
# send_rpc func_impl "$args"
# }
# # (in another process) > # receive_rpc() {
# local func="$1"
# local args="$2"
# eval $func "$args"
# }
pack_args()
{
local _p _resvar=$1 _result= _value
shift
for _p
do
_result="$_result '${_p//'/'\"'\"'}'"
done
eval $_resvar=\${_result\# }
}
Harvey
Am 14.02.24 um 19:06 schrieb Harvey:
Hello,
I am part of the team of a small router distribution called fli4l:
https://www.fli4l.de/doku.php?id=start.
We use buildroot to generate our images. Due to a hardware crash and the
leaving of our main developer we are facing the dead of our project. But
there are still some people left and at least we want to try to keep it
alive 😉
That said - the first we have to try is the update of our buildroot
base. A lot of work is already done and but we struggle with the busybox
update, especially with the ash shell contained. When updating to
Version 1.36.1 we are facing script failures in places that did work
until busybox 1.31.1.
I have tried to debug the code and it seems that the error is contained
in a helper include for string handling.
The code is:
# Packs arguments into a single string.
#
# Input:
# $1 = name of variable receiving the arguments in such a way that
# (re)evaluating them provides the original arguments
# $2... = arguments
# Example:
# func() {
# local args
# pack_args args "$@"
# send_rpc func_impl "$args"
# }
# # (in another
Re: fix large PID overflow their column in top command
On Wed, 14 Feb 2024 14:05:15 +
Matthew Chae wrote:
> Hi Bernhard,
>
> I'm sending new patch and the result of bloatcheck.
Many thanks for the updated patch!
function old new delta
display_process_list14061765+359
.rodata99721 99724 +3
--
(add/remove: 0/0 grow/shrink: 2/0 up/down: 362/0) Total: 362 bytes
textdata bss dec hex filename
1009548 165071840 1027895 faf37 busybox_old
1009910 165071840 1028257 fb0a1 busybox_unstripped
I think that's too much. For me this gives +293 bytes, still way too much.
Can you please see if it helps to retain the manual formatting of
PID PPID USER?
PS:
procps/top.c: In function ‘display_process_list’:
procps/top.c:664:1: warning: ISO C90 forbids mixed declarations and code
[-Wdeclaration-after-statement]
664 | typedef struct { unsigned quot, rem; } bb_div_t;
| ^~~
so please move your new #define PID_STR block down to right before
/* what info of the processes is shown */
In
+ int lines = (lines_rem - 1);
please drop the superfluous braces.
It is most likely not smaller to use
pid_len = strlen(make_human_readable_str(pid_max,0,0))
than to introduce this private count_digits(), i fear?
Maybe we could amortize the addition of count_digits by
reusing it elsewhere, as a follow-up.
thanks
> Can you review these and give me your thoughts?
> Just let me know if you think that the code size needs to be reduced.
>
> Br-Matthew
>
> From: Bernhard Reutner-Fischer
> Sent: Tuesday, February 13, 2024 7:16 PM
> To: Matthew Chae
> Cc: rep.dot@gmail.com ; David Laight
> ; 'Denys Vlasenko' ;
> busybox@busybox.net ; Christopher Wong
>
> Subject: Re: fix large PID overflow their column in top command
>
> On Mon, 5 Feb 2024 09:56:20 +
> Matthew Chae wrote:
>
> > Hi David,
> >
> > I'm sending an improved patch based on your comments.
> >
> > Not only does it not care about the PID_MAX value,
> > it searches all the contents to be output to recognize the required column
> > width
> > and dynamically allocates the space for PID and PPID appropriately without
> > creating a lot of empty space.
> >
> > Additionally, this patch still allows user names to be displayed up to 8
> > characters without truncation.
> >
> > Can you look through the attachment?
>
> Unfortunately the patch does not apply to current master.
> How much would your patch add to the size? Can you bring it down to a
> minimum?
> See make baseline; apply the patch;make bloatcheck
>
> thanks
>
> > (0002-Allocate-PID-PPID-space-dynamically-in-top-command.patch)
> >
> > BR-Matthew Chae
> >
> > From: David Laight
> > Sent: Thursday, November 23, 2023 6:10 PM
> > To: 'Denys Vlasenko' ; Matthew Chae
> >
> > Cc: busybox@busybox.net ; Christopher Wong
> >
> > Subject: RE: fix large PID overflow their column in top command
> >
> > ...
> > > + fp = xfopen_for_read("/proc/sys/kernel/pid_max");
> > > + if (!fgets(pid_buf, PID_DIGITS_MAX + 1, fp)) {
> > > ...
> > > + if (strncmp(pid_buf, "32768", 5) == 0)
> > > + pid_digits_num = 5;
> > > + else
> > > + pid_digits_num = PID_DIGITS_MAX;
> > >
> > > The logic above is not sound. Even if sysctl kernel.pid_max
> > > is 32768, there can be already running processes with pids > 9.
> >
> > It's also probably wrong for pretty much all other values.
> >
> > I'd just base the column width on strlen(pid_buf) with a minimum
> > value of 5.
> >
> > It is unlikely that pid_max has been reduced - so column overflow
> > it that case probably doesn't really matter.
> >
> > The more interesting case is really a system with a very large pid_max
> > that has never run many processes.
> > You don't really want lots of blank space.
> >
> > I can't remember whether top reads everything before doing any output?
> > Since the output is sorted it probably almost always does.
> > In which case it knows the column width it will need.
> >
> > I did post a patch a while back that enabled 'Irix mode'.
> > (100% cpu is one cpu at 100%, not all cpus at 100%)
> > Maybe I should dig it out again.
> >
> > David
> >
> > -
> > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1
> > 1PT, UK
> > Registration No: 1397386 (Wales)
>
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: udhcpc: throttled down data transmission
Am Thu, Feb 15, 2024 at 12:23:06PM + schrieb Laurent Bercot: > > I'm using 'udhcpc' on my ARM64 platform with the following scenario: I have > > one > > 'eth0' interface that has a static ip address and the 'udhcpc' client on the > > same interface. That leads to low data transmission performace on that > > interface. > > Most people use udhcpc when they *don't* have a static IP address on > an interface, the point of DHCP being to assign an IP address. In that > common use case, it is impossible to use UDP, since IPv4 is not > configured yet! Listening on a raw socket is the only option. > > Your use case is pretty niche, already having IPv4 configured on your > interface before you use udhcpc. It's not surprising that udhcpc isn't > optimized for it: it's a small client after all. > > I suppose it could be patched to add an option to use UDP when the > interface already has an address, but it would mean effort, and more code > in busybox, which is probably not worth the trade-off. > > -- > Laurent > Thank you for the clear explanation. I'm going to disable the 'udhcpc' client when the interface 'eth0' has already an static IP address. Best regards Alexander Wilhelm ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: udhcpc: throttled down data transmission
I'm using 'udhcpc' on my ARM64 platform with the following scenario: I have one 'eth0' interface that has a static ip address and the 'udhcpc' client on the same interface. That leads to low data transmission performace on that interface. Most people use udhcpc when they *don't* have a static IP address on an interface, the point of DHCP being to assign an IP address. In that common use case, it is impossible to use UDP, since IPv4 is not configured yet! Listening on a raw socket is the only option. Your use case is pretty niche, already having IPv4 configured on your interface before you use udhcpc. It's not surprising that udhcpc isn't optimized for it: it's a small client after all. I suppose it could be patched to add an option to use UDP when the interface already has an address, but it would mean effort, and more code in busybox, which is probably not worth the trade-off. -- Laurent ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
RE: Failing shell code under busybox 1.36.1 that worked with 1.31.1
From: David Leonard
> Sent: 15 February 2024 08:46
> To: Harvey
> Cc: busybox@busybox.net
> Subject: Re: Failing shell code under busybox 1.36.1 that worked with 1.31.1
>
>
> > _result="$_result '${_p//'/'\"'\"'}'"
>
> Busybox ash has a CONFIG_ASH_BASH_COMPAT config to get this ${var//...}
> replacement behaviour. And, in bash, the ' in the pattern section is treated
> as a quote start. To give the ' a literal meaning, use \'
> eg ${_p//\'/..repl..}
...
It is probably safer to the quotes into variables.
Then the syntax analysis won't get messed up.
So something like:
sq="'"
qsq='"'"'"'"'
then
"${_p/$sg/$qsq}"
is probably ok and maybe more readable.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
RE: Failing shell code under busybox 1.36.1 that worked with 1.31.1
From: Harvey
> Sent: 14 February 2024 18:18
> Sorry, posted the wrong version of the pack_args() funtion:
> I have payed with possible solutions for so long that I forgot the
> restore the original before posting -blush-
>
> Here is the original version:
>
>
> # Packs arguments into a single string.
> #
> # Input:
> # $1 = name of variable receiving the arguments in such a way that
> #(re)evaluating them provides the original arguments
> # $2... = arguments
> # Example:
> # func() {
> # local args
> # pack_args args "$@"
> # send_rpc func_impl "$args"
> # }
> # # (in another process)
> # receive_rpc() {
> # local func="$1"
> # local args="$2"
> # eval $func "$args"
> # }
> pack_args()
> {
> local _p _resvar=$1 _result= _value
> shift
> for _p
> do
> _result="$_result '${_p//'/'\"'\"'}'"
> done
> eval $_resvar=\${_result\# }
> }
Try just splitting the quoting of the final '
Just adding "" before it might be enough.
Failing that add it on the next line.
Probably easier than trying to find the bug in ash.
(been there, done that...)
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: Failing shell code under busybox 1.36.1 that worked with 1.31.1
_result="$_result '${_p//'/'\"'\"'}'"
Busybox ash has a CONFIG_ASH_BASH_COMPAT config to get this ${var//...}
replacement behaviour. And, in bash, the ' in the pattern section is treated
as a quote start. To give the ' a literal meaning, use \'
eg ${_p//\'/..repl..}
In current busybox ash you will run into a bug where quotes aren't
handled in the pattern, and even loses the rest of the word, as you
observed. That is, given this script:
var="don't g/o"
echo "${var/'/'o}!"
echo "${var/\'/\'o}!"
bash outputs:
don't g!(pattern="/o" repl="")
don'ot g/o! (pattern="'", repl="'o")
and busybox ash outputs:
don't g/o (pattern=?, repl=?, NOTE: lost !)
don'ot g/o! (pattern="'", repl="'o")
Here's a patch to make ash a little bit closer to bash.
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -7058,6 +7058,7 @@ subevalvar(char *start, char *str, int strloc,
*/
repl = NULL;
if (subtype == VSREPLACE || subtype == VSREPLACEALL) {
+ int quoted = 0;
/* Find '/' and replace with NUL */
repl = start;
/* The pattern can't be empty.
@@ -7073,6 +7074,11 @@ subevalvar(char *start, char *str, int strloc,
repl = NULL;
break;
}
+ /* Handle quoted patterns */
+ if ((unsigned char)*repl == CTLQUOTEMARK)
+ quoted ^= 1;
+ if (quoted)
+ goto literal;
if (*repl == '/') {
*repl = '\0';
break;
@@ -7084,6 +7090,7 @@ subevalvar(char *start, char *str, int strloc,
/* Handle escaped slashes, e.g. "${v/\//_}" (they are
CTLESC'ed by this point) */
if ((unsigned char)*repl == CTLESC && repl[1])
repl++;
+ literal:
repl++;
}
}
new file mode 100644
index 0..23aecb5ae
--- /dev/null
+++ b/shell/ash_test/ash-vars/var_bash_repl_quoted.right
@@ -0,0 +1,3 @@
+ab.
+axb.
+x/b.
new file mode 100644
index 0..d2ba39578
--- /dev/null
+++ b/shell/ash_test/ash-vars/var_bash_repl_quoted.tests
@@ -0,0 +1,4 @@
+v=a/b
+echo "${v/'/'}."
+echo "${v/'/'/x}."
+echo "${v/'a'/x}."
On Wed, 14 Feb 2024, Harvey wrote:
Sorry, posted the wrong version of the pack_args() funtion:
I have payed with possible solutions for so long that I forgot the
restore the original before posting -blush-
Here is the original version:
# Packs arguments into a single string.
#
# Input:
# $1 = name of variable receiving the arguments in such a way that
#(re)evaluating them provides the original arguments
# $2... = arguments
# Example:
# func() {
# local args
# pack_args args "$@"
# send_rpc func_impl "$args"
# }
# # (in another process) > # receive_rpc() {
# local func="$1"
# local args="$2"
# eval $func "$args"
# }
pack_args()
{
local _p _resvar=$1 _result= _value
shift
for _p
do
_result="$_result '${_p//'/'\"'\"'}'"
done
eval $_resvar=\${_result\# }
}
Harvey
Am 14.02.24 um 19:06 schrieb Harvey:
Hello,
I am part of the team of a small router distribution called fli4l:
https://www.fli4l.de/doku.php?id=start.
We use buildroot to generate our images. Due to a hardware crash and the
leaving of our main developer we are facing the dead of our project. But
there are still some people left and at least we want to try to keep it
alive 😉
That said - the first we have to try is the update of our buildroot
base. A lot of work is already done and but we struggle with the busybox
update, especially with the ash shell contained. When updating to
Version 1.36.1 we are facing script failures in places that did work
until busybox 1.31.1.
I have tried to debug the code and it seems that the error is contained
in a helper include for string handling.
The code is:
# Packs arguments into a single string.
#
# Input:
# $1 = name of variable receiving the arguments in such a way that
# (re)evaluating them provides the original arguments
# $2... = arguments
# Example:
# func() {
# local args
# pack_args args "$@"
# send_rpc func_impl "$args"
# }
# # (in another process)
# receive_rpc() {
# local func="$1"
# local args="$2"
# eval $func "$args"
# }
pack_args()
{
local _p _resvar=$1 _result= _value
shift
for _p
do
_result="$_result '${_p//'/'\"'\"'}''"
