Re: [Qemu-devel] [PATCH 1/5] target-arm: Add ARM CPU feature parsing

["Alex Bennée" <alex.bennee@linaro.org>]

Greg Bellows <greg.bellows@linaro.org> writes:

> Thanks Alex comments inline....
>
<snip>
>>
>> Aren't we leaking here? strtok returns the next token (or NULL) so don't
>> we loose the original ptr?
>>
>>
> ​As I understand it, strtok uses static pointers to track the location
> within an existing string rather than allocating storage that the user must
> free.  This is apparently what makes the version I used non-reentrant.​  In
> which case, there should not be an leak due to its use.

Yeah - I realised this after re-reading the man page. Non-re-entrant
isn't a particular problem these days but it still feels dirty....

>> Also while using glib you might want to consider using glib's own
>> tokenising functions (e.g. g_strsplit). This has the advantage of having
>> helper functions like g_strfreev() which can clean-up everything in one go.
>>
>
> ​I certainly can use the glib version, but in this case I did not see the
> advantage. In fact, it actually may be less performant​ to use the glib
> version given it needs to allocate/free memory.  I am fine either way if
> anyone feels strongly.

I suspect this discussion is trumped by moving to the feat_foo=on/off
parsing style referenced elsewhere so we can use common code.

-- 
Alex Bennée