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

[Ján Tomko]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150513/13f74093/attachment-0001.sig>