On Mon, Jan 25, 2016 at 09:00:27PM -0500, Laine Stump wrote:
> >> + retries = MACVLAN_MAX_ID + 1;
> >> + while (!ifnameCreated && retries) {
> >> virMutexLock(&virNetDevMacVLanCreateMutex);
> >> - for (c = 0; c < 8192; c++) {
> >> - snprintf(ifname, sizeof(ifname), pattern, c);
> >> - if ((ret = virNetDevExists(ifname)) < 0) {
> >> - virMutexUnlock(&virNetDevMacVLanCreateMutex);
> >> + reservedID = virNetDevMacVLanReserveID(reservedID, flags, false, true);
> >> + if (reservedID < 0) {
> >> + virMutexUnlock(&virNetDevMacVLanCreateMutex);
> >> + return -1;
> >> + }
> >> + snprintf(ifname, sizeof(ifname), pattern, reservedID);
> > Since you're changing this, snprintf returns -1 in case of error.
>
> Well, the man page says -1 is returned by the *printf functions "if an
> output error is encountered", which would include an I/O error (moot for
> snprintf() since it doesn't do any I/O) or a bad format string (but
> we're calling it with a literal format string that only includes a
> single %d, so it is known to be good). Checking the returned length is
> pointless too, since by definition the longest possible result of
> macvtap%d where the %d is guaranteed between 0 and 8192 is
> "macvtap8192", which is 12 characters - safely below the limit of
> IFNAMSIZ (16).
Sure, I've looked at the man page and also into gnulib code and just wanted to
point it out, that we may want to somehow handle -1, but if it happens probably
the whole libvirt is going to die with OOM. Let's leave it as it is.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list