[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/5] target-arm: Add ARM CPU feature parsing
From: |
Alex Bennée |
Subject: |
Re: [Qemu-devel] [PATCH 1/5] target-arm: Add ARM CPU feature parsing |
Date: |
Wed, 21 Jan 2015 10:57:04 +0000 |
Greg Bellows <address@hidden> 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
- [Qemu-devel] [PATCH 0/5] target-arm: ARM64: Adding EL1 AARCH32 guest support, Greg Bellows, 2015/01/19
- [Qemu-devel] [PATCH 1/5] target-arm: Add ARM CPU feature parsing, Greg Bellows, 2015/01/19
- Re: [Qemu-devel] [PATCH 1/5] target-arm: Add ARM CPU feature parsing, Igor Mammedov, 2015/01/20
- Re: [Qemu-devel] [PATCH 1/5] target-arm: Add ARM CPU feature parsing, Peter Maydell, 2015/01/20
- Re: [Qemu-devel] [PATCH 1/5] target-arm: Add ARM CPU feature parsing, Igor Mammedov, 2015/01/20
- Re: [Qemu-devel] [PATCH 1/5] target-arm: Add ARM CPU feature parsing, Peter Maydell, 2015/01/20
- Re: [Qemu-devel] [PATCH 1/5] target-arm: Add ARM CPU feature parsing, Igor Mammedov, 2015/01/20
- Re: [Qemu-devel] [PATCH 1/5] target-arm: Add ARM CPU feature parsing, Greg Bellows, 2015/01/20
- Re: [Qemu-devel] [PATCH 1/5] target-arm: Add ARM CPU feature parsing, Igor Mammedov, 2015/01/21