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') > > Jan >> >> ACK with that simplification. at which point p is no longer a dead variable, but I've still reduced the code complexity. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list