Hi Steffen,
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
> 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?
> +
> +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.
> diff --git a/shell/shexp-arith.h b/shell/shexp-arith.h
> new file mode 100644
> index 0000000000..5f3655b456
> --- /dev/null
> +++ b/shell/shexp-arith.h
> @@ -0,0 +1,1292 @@
> +/*@ S-nail - a mail user agent derived from Berkeley Mail.
Missing license statement here?
> + *@ Signed 64-bit sh(1)ell-style $(( ARITH ))metic expression evaluator.
> + *@ POW2 bases are parsed as unsigned, operation overflow -> limit
> constant(s),
> + *@ saturated mode is not supported, division by zero is handled via error.
> + *@ The expression length limit is ~100.000.000 on 32-bit, U32_MAX otherwise.
> + *@ After reading on Dijkstra's two stack algorithm, as well as bash:expr.c.
> + *@ Most heavily inspired by busybox -- conclusion: the Dijkstra algorithm
> + *@ scales very badly to ternary as are used to implement conditionals and
> + *@ their ignored sub-expressions.
> + *@
> + *@ #define's:
> + *@ - a_SHEXP_ARITH_COMPAT_SHIMS: for inclusion in other code bases, setting
> + *@ this defines most necessary compat macros.
these defines
> + *@ We still need s64, u64, S64_MIN, savestr(CP) <> strdup(3) that does not
> + *@ return NIL (only with _ERROR_TRACK). Plus stdint.h, ctype.h, string.h.
> + *@ We need su_idec_cp(), su_ienc_s64(), n_var_vlook() and n_var_vset().
> + *@ We need su_IDEC_STATE_EMASK (= 1) and su_IDEC_STATE_CONSUMED (= 2),
> e.g.:
> + *@ errno = 0;
> + *@ res = strto_arith_t(cbuf, (char**)endptr_or_nil);
> + *@ rv = 0;
> + *@ if(errno == 0){
> + *@ if(**endptr_or_nil == '\0')
> + *@ rv = su_IDEC_STATE_CONSUMED;
> + *@ }else{
> + *@ rv = su_IDEC_STATE_EMASK;
> + *@ res = 0;
> + *@ }
> + *@ *S(s64*,resp) = res;
> + *@ - a_SHEXP_ARITH_COOKIE: adds struct a_shexp_arith_ctx:sac_cookie, and
> + *@ a cookie arg to a_shexp_arith_eval().
> + *@ - a_SHEXP_ARITH_ERROR_TRACK: add "char **error_track_or_nil" to
> + *@ a_shexp_arith_eval(), and according error stack handling, so that users
> + *@ can be given hint where an error occurred. ("Three stack algorithm.")
> + *
> + * Copyright (c) 2022 Steffen Nurpmeso <[email protected]>.
> + * SPDX-License-Identifier: ISC
> + *
> + * Permission to use, copy, modify, and/or distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
Ah, here is the license. Please move it to line 2 ?
> +static void
> +a_shexp__arith_eval(struct a_shexp_arith_ctx *self,
> + char const *exp_buf, uz exp_len){
> + char *ep, *varp, *cp, c;
> + u16 lop;
> + struct a_shexp_arith_stack *sasp;
> + void *mem_p;
> + NYD2_IN;
What's NYD2_IN supposed to accomplish? Delete.
> +
> + a_SHEXP_ARITH_L((" > _arith_eval %zu <%.*s>\n",
> + exp_len, S(int,exp_len != UZ_MAX ? exp_len : su_cs_len(exp_buf)),
> + exp_buf));
> +
> + mem_p = NIL;
> + sasp = self->sac_stack;
> +
> + /* Create a single continuous allocation for anything */
> + /* C99 */{
> + union {void *v; char *c;} p;
> + uz i, j, a;
> +
> + /* Done for empty expression */
> + if((i = a_shexp__arith_ws_squeeze(self, exp_buf, exp_len, NIL)) == 0)
> + goto jleave;
> + ++i;
> +
> + /* 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 ;)
What was the initial expression that did not work properly with the old
implementation?
thanks,
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
