Re: [Qemu-devel] [PATCH v2 6/7] libcacard/vcard_emul_nss: Assert vreaderOpt isn't null

[Michael Tokarev]

23.05.2014 22:09, Michael Tokarev wrote:
> 23.05.2014 15:24, Markus Armbruster wrote:
>> It's not locally obvious, and Coverity can't see it either.
>>
>> Signed-off-by: Markus Armbruster <address@hidden>
>> Reviewed-by: Alon Levy <address@hidden>
>> ---
>>  libcacard/vcard_emul_nss.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c
>> index 2048917..4f55e44 100644
>> --- a/libcacard/vcard_emul_nss.c
>> +++ b/libcacard/vcard_emul_nss.c
>> @@ -1181,6 +1181,7 @@ vcard_emul_options(const char *args)
>>                  vreaderOpt = g_renew(VirtualReaderOptions, opts->vreader,
>>                                       reader_count);
>>              }
>> +            assert(vreaderOpt);
>>              opts->vreader = vreaderOpt;
>>              vreaderOpt = &vreaderOpt[opts->vreader_count];
>>              vreaderOpt->name = g_strndup(name, name_length);
> 
> Shouldn't the assignment be moved up one line into the if {}
> statement instead?

Actually it looks like this code is just buggy, it works for just
one iteration.  Because at this place, vreaderOpts will be non-NULL
only if we expanded the array.  If we didn't (we do that in
READER_STEP increments), we'll be assigning NULL to opts->vreader,
because vreaderOpt will _really_ be NULL here.

One good example when setting initial values like this (for vreaderOpts)
is a Bad Idea (tm).  If it weren't needlessly NULL initially, compiler
was able to tell us about using uninitialized variable.

Thanks,

/mjt