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

Andy Chu andychup at gmail.com
Tue Mar 15 14:46:36 PDT 2016


> If it's at the end of expr_main() we don't care. However, if leaks
> per-input and we have unlimited input... (I believe the current linux
> environment space limit is INT_MAX arguments each INT_MAX long, modulo
> ulimit applying something to that maybe?)
>
> This sounds per-argument, so I'd want to free it when it's done
> with it if we can. (I need to read the code after applying your
> patches.)

OK great, some notes on memory management for when you review:

expr is a pure function of the argv, and the implementation almost
always uses the constant argv strings, which we don't need to manage.
The two exceptions are:

- re() -- when you do expr foo : 'f\(.*\)', the resulting captured
string "oo" has to go somewhere
- type coercion of integers: if you do expr 2 + 3 \< abcd  -- 5 on the
LHS is converted to "5" by get_str(), called from eval_op().

So if you don't do any of those things, you won't get any leaks.  In
the worst case, you could have one leak for every 3 or 4 argv entries
(you can't make it leak for every entry AFAICT).  And the captured
string is smaller than the length of a single arg, which is limited to
131072 bytes on Linux I think.

http://www.in-ulm.de/~mascheck/various/argmax/

In practice I don't think it will matter because I don't think people
are really writing huge expressions with expr, because it will get
unreadable.  It has no intermediate variables -- it would have to be a
single huge expression that does tons of regex captures or mixes types
a lot.

I was thinking to clean up in expr_main -- cleaning up more
aggressively would be a little tricky since get_str() only allocates
conditionally.  But I guess you're saying that isn't worth it since
the process will be torn down.

And yes it definitely helps to have code in pending, so I'm all for
putting unfinished / untested stuff there!  (Though what I'm surprised
by is that I started up ConnectBot on my Nexus 5X phone and was able
to tickle bugs which already have failing test cases!  Like the
precedence bugs in expr.)

thanks,
Andy


More information about the Toybox mailing list