[Toybox] [PATCH] Fix the operator precedence in expr.

[Andy Chu]

Here is a second patch on top of the first one.  Between these two
patches, it's pretty much a complete rewrite -- different algorithm,
data structures, and code structure.

It's back at 276 lines, which I think compares pretty favorably to
busybox at 538 lines:

http://code.metager.de/source/xref/busybox/coreutils/expr.c

One thing is that I don't quite understand the memory management
strategy.  I don't know exactly what CFG_TOYBOX_FREE is for.  The code
already had two issues:

- xmprintf in re() leaks I think
- in num_to_str, the same static buffer was used for multiple strings,
which is a latent bug because you could strcmp() the same buffer
against itself

I changed it to use xmalloc, which will cause a memory leak too.  I
guess if I want to clean up properly, I should keep a list of pointers
in GLOBALS and free them all at the end in expr_main() ?

BTW this 0001- patch is more up to date than the one in the previous
e-mail (I used my @google.com e-mail address because my employer wants
that -- it's stricter now due to all the lawsuits...)

Andy

-----

[PATCH 2/2] Fix type coercion bugs in expr.

All tests pass now; this fixes the 2 remaining failures, including a
segfault.

The structure of the code has changed a lot -- instead of having a tiny
function per operator, we have eval_op() which does common type coercion
and then evaluates the operator.  I tried writing it a couple different
ways, and this was the cleanest.

The OPS table now contains the operator string, precedence level,
signature for type coercion, and operator ID.

On Mon, Mar 14, 2016 at 9:29 PM, Andy Chu <andychup at gmail.com> wrote:
> This fixes 3 tests, and the delta in expr.c is +11 lines -- all
> comments.  The actual code is shorter.
>
> It's a bit unnatural to me but I tried to compress the code, following
> the existing style.
>
> I'm going to fix the type coercion problems too, which will piggyback
> on this change.  Basically the OPS lookup table needs have another
> field to specify coercion rules for the operation.  The type
> conversion has to be done with respect to the operation being
> evaluated rather than globally up front.
>
> # fails with type error, '21' should be coerced to integer and then added to 3
> expr ab21xx : '[^0-9]*\([0-9]*\)' + 3
>
> # segfaults because '3' is coerced to integer at parse time, and we
> pass NULL to regexec
> expr 3 : '\(.\)'
>
> -----
>
> expr now uses the precedence table specified by POSIX, implemented using
> the "precedence climbing" algorithm.  See the references at the top of
> eval_expr().
>
> This fixes 3 of 4 failing tests.
>
> I also added more tests for correct behavior and for syntax errors.
> This includes a new test exposing a segfault, related to type coercion.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Fix-the-operator-precedence-in-expr.patch
Type: text/x-patch
Size: 8631 bytes
Desc: not available
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20160315/39af5c4e/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Fix-type-coercion-bugs-in-expr.patch
Type: text/x-patch
Size: 13851 bytes
Desc: not available
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20160315/39af5c4e/attachment-0005.bin>