[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [Qemu-devel] [trivial for-2.6] util/id: fully allocat
From: |
John Snow |
Subject: |
Re: [Qemu-trivial] [Qemu-devel] [trivial for-2.6] util/id: fully allocate names table |
Date: |
Wed, 25 Nov 2015 10:17:54 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 11/25/2015 03:18 AM, Markus Armbruster wrote:
> John Snow <address@hidden> writes:
>
>> Trivial: this array should be allocated to have ID_MAX entries always.
>> Otherwise if someone were to forget to expand this table, the assertion
>> in the id generator won't actually trigger; it will read junk data.
>
> You mean this one:
>
> assert(id < ID_MAX);
>
Well, sort of. I meant 'assert(id_subsys_str[id])' itself. If you forget
to expand the list (It happened to a friend of mine) this assert will
pass because it reads garbage.
If you just always expand the full table, though, it will catch you
(Err, my friend) being a dummy a little more nicely.
My thought is we need both the range and presence checks.
I'll v2 it, thanks.
--js
> The assertion is crap, because it fails to protect array access
> id_subsys_str[id]. Here's one that does:
>
> assert(0 <= id && id < ARRAY_SIZE(id_subsys_str));
>
>> Signed-off-by: John Snow <address@hidden>
>> ---
>> util/id.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/util/id.c b/util/id.c
>> index bcc64d8..b7ca4d2 100644
>> --- a/util/id.c
>> +++ b/util/id.c
>> @@ -29,7 +29,7 @@ bool id_wellformed(const char *id)
>>
>> #define ID_SPECIAL_CHAR '#'
>>
>> -static const char *const id_subsys_str[] = {
>> +static const char *const id_subsys_str[ID_MAX] = {
>> [ID_QDEV] = "qdev",
>> [ID_BLOCK] = "block",
>> };