I'll make my commit messages more detailed.
The strdup() is necessary. Due to the string/integer duality of the
values, integer values must be coerced into strings on occasion. This
is done with the valstr() function which will print the integer into
the buffer given. In this case that buffer is local to the match()
function.
If vstr contained a string, then sure we could just return (Val){ s, 0
}; and everything is fine. But if vstr contained an integer, it is
changed to a string stored in buf1 and s points into buf1. In this
case if we did as above, the pointer returned points into buf1 which
has local storage (colloquially "on the stack"). After match() returns
this pointer would be invalid.
I have yet to think of a good way to avoid this problem, if one comes
to mind please let me know and/or just go fix it. If we do keep the
strdup() and want to be able to free() it then I'd probably just
create a global list/stack/something and add allocations to it so they
can be free()d after successful execution.
Ok, while writing this email I went ahead and did that, the diff is
attached. If you think it's worth the extra code for correctness go
ahead and commit it. It's not too much code, but it's also a small
problem. I'm torn...
-emg
On Thu, Feb 26, 2015 at 12:14 PM, Dimitris Papastamos <sin_AT_2f30.org> wrote:
> On Wed, Feb 25, 2015 at 08:20:18PM -0800, Evan Gates wrote:
>> Went back and added some comments to expr where I thought it would
>> benefit from extra explanation. Got rid of unnecessary allocations.
>> Used utfnlen() with the match operator to add UTF-8 support. Made some
>> changes for the style guide, then also rearranged a few things that
>> IMO make the code more readable.
>>
>> I added a FIXME? comment. There is a strdup() that is never free()d.
>> Is it worth keeping track of it at a global level just to free when
>> we're done?
>
> BTW, it would have been useful to include the text above in the commit
> message :)
>
> I don't think we should bother with that strdup() at all.
>
Received on Thu Feb 26 2015 - 22:46:23 CET