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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list