On Wed, May 13, 2015 at 06:39:12AM -0400, John Ferlan wrote: > > > 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). strchr does not return NULL here because networkFindUnusedBridgeName is only called if either def->bridge is NULL or def->bridge contains "%d". Jan > 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
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list