Hello.
Bernhard Reutner-Fischer wrote in
<20220906183821.1f82672d@nbbrfq>:
|On Wed, 31 Aug 2022 01:43:26 +0200
|Steffen Nurpmeso <[email protected]> wrote:
|
|> The former implementation was not correct regarding whiteouts in
|> ?: conditional branches. The new one also parses a bit better, in
|> effect on equal level than bash with its recursive descendent parser.
|
|Please provide bloat-o-meter output.
|i.e.
|make baseline
|# apply patch
|make bloatcheck
That made me find docs/keep_data_small.txt.
With my own busybox .config (very much) against the same with also
CONFIG_FEATURE_SH_MATH_ERROR_TRACK=y, eh.. i get
/usr/bin/env: ‘python’: No such file or directory
make: *** [/x/src/busybox.git/Makefile.custom:112: bloatcheck] Error 127
There is only python3 here. I get
#?0|kent:busybox.git$ make bloatcheck
function old new delta
static.a_shexp__arith_eval - 1891 +1891
a_shexp__arith_op_apply - 1532 +1532
a_shexp__arith_val_eval - 272 +272
arith 14 264 +250
a_shexp_arith_op_toks - 199 +199
strto_arith_t - 156 +156
static.a_shexp__arith_ws_squeeze - 147 +147
.rodata 104665 104750 +85
a_shexp__arith_op_apply_colons - 75 +75
ash_arith 126 123 -3
op_tokens 141 - -141
arith_lookup_val 193 - -193
arith_apply 1044 - -1044
evaluate_string 1146 - -1146
------------------------------------------------------------------------------
(add/remove: 7/4 grow/shrink: 2/1 up/down: 4607/-2527) Total: 2080 bytes
text data bss dec hex filename
1926334 32428 29370 1988132 1e5624 busybox_old
1928414 32428 29370 1990212 1e5e44 busybox_unstripped
|> diff --git a/shell/math.c b/shell/math.c
|> index 76d22c9bd5..8ba0d2f7fb 100644
|> --- a/shell/math.c
|> +++ b/shell/math.c
|
|> +#define su_ienc_s64(X,Y,Z) (sprintf(X, ARITH_FMT, Y), X)
|> +#define n_var_vlook(X,Y) (*self->sac_cookie->lookupvar)(X)
|> +#define n_var_vset(X,Y,Z) (*self->sac_cookie->setvar)(X, (char*)(Y))
|> +#define su_idec_cp(A,B,C,D,E) a_idec_x(A, B, E)
|
|Can you drop the unused parameters?
Well if you busyboxify shexp-arith.h then yes.
|> +static inline uint32_t a_idec_x(void *resp, char const *cbuf,
|> + char const **endptr_or_nil){
|> + uint32_t rv;
|> + arith_t res;
|> + char const *eptr;
|> +
|> + if(endptr_or_nil == NULL)
|> + endptr_or_nil = &eptr;
|
|Space after if please.
Below.
...
|> +++ b/shell/shexp-arith.h
|> @@ -0,0 +1,1292 @@
|> +/*@ S-nail - a mail user agent derived from Berkeley Mail.
|
|Missing license statement here?
That is not always in line 2 for busybox, too.
...
|> + *@ - a_SHEXP_ARITH_COMPAT_SHIMS: for inclusion in other code bases, \
...
|these defines
Yes?
...
|Ah, here is the license. Please move it to line 2 ?
...
|What's NYD2_IN supposed to accomplish? Delete.
So what is this now? I said shexp-arith.h is taken as-is from my
mailer for which i wrote it. It could remain so, hooked in via
compatibility shims, or the code may be moved entirely to
busybox's math.c, with syntax adjustments just as you desire.
In this case i would expect the above bloatcheck to become
less of a headache. A bit at least.
NYD is part of my infrastructure that is used in shexp-arith.h.
|> + /* Overflow check: since arithmetic expressions are rarely \
|> long enough
|> + * to come near this limit, xxx laxe & fuzzy, not exact; max \
|> U32_MAX! */
|> + if(su_64( i > U32_MAX || ) i >= UZ_MAX / 2 ||
|
|I have to admit that the amount of macro maze makes it really hard to
|read ;)
Well it is easier than having lots of #ifdef #else #endif blocks
in my opinion. On 32-bit the above would spit out a warning
(possibly, i have not really looked what compiler/linker flags
busybox uses) because i>U32_MAX can never be true. I mean i could
say i>=U32_MAX-1 or what, sure. The above is "laxe and fuzzy"
anyhow. :-)
Btw if you start reading after the /* -- >8 -- 8< -- */ scissor
line it is quite in a flow. (I usually do not use preprocessor
macro in files, the support uses quite a lot, though.)
I mean, i have a little infrastructure, which is used here.
The pure part of it, and i am hoping the mailer part of it, to
which shexp-arith.h belongs, in a distant future, too, can be
compiled with a C++ compiler also.
Regarding overall syntax: is there a formatter support file you
could use if you integrate it?
|What was the initial expression that did not work properly with the old
|implementation?
Well if you run the test of the patchset against busybox with the
old implementation you will see multiple errors (of course >>>[=]
and **= because they are not supported), mostly ?: related. To
make it plain, ?: is broken but for simple cases.
With the patch applied busybox is en par with bash.
Even slightly "better". (Though it was a conscious decision not
to improve the reported behaviour, because "bash is already doing
more than necessary", more or less correctly quoted.)
--steffen
|
|Der Kragenbaer, The moon bear,
|der holt sich munter he cheerfully and one by one
|einen nach dem anderen runter wa.ks himself off
|(By Robert Gernhardt)
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
