[libvirt] [PATCH 2/6] virDomainFormatSchedDef: Handle case when func returns NULL

[Michal Privoznik]

On 31.05.2016 13:48, Peter Krempa wrote:
> On Tue, May 31, 2016 at 13:32:50 +0200, Michal Privoznik wrote:
>> On 31.05.2016 13:10, Peter Krempa wrote:
>>> On Tue, May 31, 2016 at 13:05:16 +0200, Michal Privoznik wrote:
>>>> On 31.05.2016 12:52, Peter Krempa wrote:
>>>>> On Tue, May 31, 2016 at 12:33:26 +0200, Michal Privoznik wrote:
>>>>>> As unlikely as it might seem, func passed to this function can
>>>>>> return NULL. And in some cases it indeed does so:
>>>>>> virDomainDefGetVcpuSched and virDomainDefGetIOThreadSched.
>>>>>> However, the function I'm fixing blindly dereference value func
>>>>>> returned thus leading to SIGSEGV.
>>>>>
>>>>> Same as in the coverity case complaining about this it won't happen here
>>>>> since we are iterating a known list of items that was collected
>>>>> beforehand and thus no crashing will happen.
>>>>>
>>>>
>>>> Well, having covery reports a false positive is one thing, being unable
>>>> to compile is another one. This is what I was afraid of and expressed it
>>>> here:
>>>>
>>>> https://www.redhat.com/archives/libvir-list/2016-May/msg02063.html
>>>>
>>>> On the other hand, you use more recent compiler than I do, so you'll hit
>>>> these error sooner O:-) IOW, one day we will have to deal with this. Again.
>>>
>>> I'm thinking of disabling that warning at all since it's making us work
>>> it around in cases where it clearly doesn't make sense. It's very
>>> similar to the warning of comparison between signed and unsigned.
>>>
>>
>> Is it? What about the following code:
> 
> Yes.
> 

Not quite. Is is only because of this warning being enabled that I've
found two bugs fixed in this series.

>>
>> char *c = NULL;
>>
>> *c = 'a';
> 
> How about:
> 
> int a = -1;
> size_t i;
> 
> for (i = 0; i < a; i++)
>    do_stuff(i);

There is no bug in here. In my example above, there's a real bug.
Therefore I find these two distinct.

Michal