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

[Rob Landley]

On 03/16/2016 01:34 PM, Andy Chu wrote:
>>   if (p<*toys.optargs || p>toys.optargs[toys.optc-1]) not_argv();
>>
>> It would be nice to free this data ourselves because expr is close to
>> $((1+2)) in the shell, and it would be nice if the shell could just
>> internall call expr nofork in that case.
> 
> I attached to a patch to track the strings allocated and then free
> them at the end.

I've already started cleanup on this command, but I'll integrate this
with what I've got.

> (Note that there is NOT a "node" per argument ('struct value' which
> which contains an int or string).  The nodes are on the stack and used
> destructively in the style of a *= b, a += b, etc.  What we are
> talking about is the strings that the nodes (may) point to.  These are
> almost always argv strings which don't need to be freed, but in the 2
> cases I mentioned, we allocate them.

I noticed that during review earlier today, but hadn't gotten to the
point of doing anything about that yet. :)

> So now this is fixed, and I tested it with ASAN (with LLVM 3.8 which
> can be downloaded easily).  It easily finds the memory leak before
> (only the regex test cases where we allocate memory fail, not every
> test case) and confirms they pass after the patch.
> 
> ASAN also found another existing memory leak in the code!  (not
> calling regfree())  When I'm triaging the tests I'll definitely see
> which ones pass under ASAN too.

Sounds like fun. I hope to have this done and promoted for the next
release. (Ballpark end of April.)

> Andy

Rob