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.