[libvirt] [PATCHv2] Ignore bridge template names with multiple printf conversions

[John Ferlan]

On 05/05/2015 12:39 PM, Eric Blake wrote:
> On 05/05/2015 10:26 AM, Ján Tomko wrote:
>> On Tue, May 05, 2015 at 10:14:18AM -0600, Eric Blake wrote:
>>> On 05/05/2015 10:05 AM, Ján Tomko wrote:
>>>> For some reason, we allow a bridge name with %d in it, which we replace
>>>> with an unsigned integer to form a bridge name that does not yet exist
>>>> on the host.
>>>>
> 
>>>> +    if (def->bridge &&
>>>> +        (p = strchr(def->bridge, '%')) == strrchr(def->bridge, '%') &&
>>>> +        strstr(def->bridge, "%d"))
>>>
>>> Simpler as:
>>>
>>> if (def->bridge &&
>>>     strstr(def->bridge, "%d") == strrchr(def->bridge, '%'))
>>
>> I still don't see it.
>>
>> [A] strchr(def->bridge, '%')
>> [B] strrchr(def->bridge, '%')
>> [C] strstr(def->bridge, "%d"))
>>
>> When def->bridge is '%s%s%s%d', [A] points to the first %s, [B] points
>> to the %d and so does [C]
>>
>> This string would pass the simplified condition (B == C), but not the
>> full one (A != C)
> 
> Okay, I see your counterargument.  Still, strstr() is pretty expensive
> compared to just:
> 
> if (def->bridge &&
>     (p = strchr(def->bridge, '%')) == strrchr(def->bridge, '%') &&
>     p[1] == 'd')
> 

Coverity complains :

Event returned_null: 	"strchr" returns null (checked 273 out of 288 times).
Event var_assigned: 	Assigning: "p" = null return value from "strchr".
Event cond_true: 	Condition "(p = strchr(def->bridge, 37)) == strrchr(def->bridge, 37)", taking true branch
Event dereference: 	Dereferencing a null pointer "p".

John
>>
>> Jan
>>>
>>> ACK with that simplification.
> 
> at which point p is no longer a dead variable, but I've still reduced
> the code complexity.
> 
> 
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>