http://bugs.winehq.org/show_bug.cgi?id=6767
--- Comment #6 from jmmikkel@mit.edu 2007-11-26 23:47:02 --- Hi, I apologize for having dropped the ball back when. Then, this past June I set out to deal with this and found that the code had changed. Unfortunately the current code will still not work correctly, and the application I was using when this problem came up for me no longer worked but for a different reason (it does not even get this far). I didn't have another application that used this functionality to try with, and I have not managed to sort through it since then.
(I do use the pkgsrc build, although it would appear 0.9.39 compiled without pkgsrc patches in June.)
The problem with the current version is that NetBSD places the required length in ifc->ifc_len at all times. So it will always have the same value two ioctl calls in a row, even if the provided buffer is still too small the second time.
Because I don't have an easy way to test this with an actual application in Wine, I have copied the enumIPAddresses function to a separate file, and added some printing, so I can test with just that. When I run the existing code, I find that ifc->ifc_len starts at 128, and the ioctl sets it to 352. Since ifc->ifc_len != lastlen (352 != 0) it goes through the loop again, setting lastlen to 352 and ifc->ifc_len to 256. The ioctl once again sets it to 352 and the loop terminates, but it shouldn't have, because 352 > 256 so my list will be truncated.
I don't know why it was changed to do the "two in a row" test. I can't think of a way for the ioctl to work that would make that test necessary instead of something more like the >= test I originally suggested. The ioctl can put three kinds of numbers in ifc_len. Numbers smaller than the provided buffer indicate the total length of the data, including zero. This means that for cases where the buffer is not big enough, there are only two possible choices: 1.) ioctl() returns the buffer size if the buffer is too short (Linux behavior, IIRC) 2.) ioctl() returns a number bigger than the size if the buffer is too short (NetBSD -- we actually need not assume the number is accurate).
The twice-in-a-row solution we currently have always results in an extra trip through the loop unless the actual length of the interface list is zero, because lastlen starts out at zero. Consider a required length of 100. lastlen is set to 0, the buffer is allocated with size 128, and the ioctl sets the required length to 100. This is != 0 so we loop again, setting lastlen to 100 and allocating a buffer of size 256. The ioctl sets the length to 100 again and the loop terminates, having run twice instead of once.
At any rate, the solution certainly covers situation 1: 128 is < 352 so the ioctl sets the length to 128, and the loop sets lastlen to 128. The ioctl is called with a buffer of 256 and ioctl sets the length to 256, which is != 128, so we loop again and call with a buffer of 512. The ioctl sets the length to 352, which is != 256, and we call the ioctl again with a buffer of 1024 but then terminate.
The twice-in-a-row solution doesn't cover situation 2 if the required length is
256 as I described above. It works okay when the length is less than 256.
Perhaps this is the situation that was addressed with the change to the current algorithm? Oddly, the solution also works if the ioctl does something strange like fill in the provided size + 1 when the buffer is not big enough (buffer size 128 -> ifc_len set to 129, buffer size 256 -> ifc_len set to 257, buffer size 512 -> ifc_len set to 352, buffer size 1024 -> ifc_len set to 352). But it breaks if the ioctl is *consistent* about the size.
A >= test also covers situation 1, at the price of an extra trip through the loop but only when the required buffer size is exactly what was provided. But that corner case required an extra trip in the first version I worked with too (using the == test you can see in the diff in the initial report). If 352 is required and 128 is provided, the ioctl sets the length to 128 and 128 >= 128 so the next loop the buffer size is 256 and the ioctl sets it to 256 and 256 >= 256 so the next loop the buffer size is 512 and the ifc_len is set to 352, which is not >= 512 and the loop terminates.
The >= test covers situation 2 as well, including for required buffer lengths > 256, of course, since it's the fix that worked for me. For NetBSD we would call the ioctl with a buffer size of 128, the ifc_len would be set to 352, and 352
= 128 so the next time through the loop would call ioctl with a size of 256,
ifc_len would be set to 352, which is >= 256, so the next loop the buffer size is 512 and the ioctl sets ifc_len to 352, which is not >= 512.
This also works with the strange version of the ioctl that returns the size + 1 when the provided size is too small: buffer size 128 -> ifc_len set to 129 and 129 >= 128 so now buffer size 256 -> ifc_len 257 and 257 >= 256 so then buffer size 512 -> ifc_len set to 352, which is not >= 512 and the loop terminates.
So my suggestion would be to return the loop back to how it was a year ago but with >= instead of == (removing lastlen):
ret = NO_ERROR; ifc->ifc_len = 0; ifc->ifc_buf = NULL; /* there is no way to know the interface count beforehand, so we need to loop again and again upping our max each time until returned < max */ do { HeapFree(GetProcessHeap(), 0, ifc->ifc_buf); if (guessedNumAddresses == 0) guessedNumAddresses = INITIAL_INTERFACES_ASSUMED; else guessedNumAddresses *= 2; ifc->ifc_len = sizeof(struct ifreq) * guessedNumAddresses; ifc->ifc_buf = HeapAlloc(GetProcessHeap(), 0, ifc->ifc_len); ioctlRet = ioctl(fd, SIOCGIFCONF, ifc); } while (ioctlRet == 0 && ifc->ifc_len >= (sizeof(struct ifreq) * guessedNumAddresses));
But if the twice-in-a-row test is really necessary for some reason I'm not seeing here, we could perhaps patch up NetBSD with the following diff to the current version, which will take advantage of the provided required length without changing the algorithm for OSes that leave the provided buffer size as the length when it's long enough. It seems more hackish to me, though (but it does eliminate the extra trip through the loop when the required size is >= 128!).
--- dlls/iphlpapi/ifenum.c.orig +++ dlls/iphlpapi/ifenum.c @@ -695,6 +695,9 @@ else guessedNumAddresses *= 2; ifc->ifc_len = sizeof(struct ifreq) * guessedNumAddresses; + if (lastlen > ifc->ifc_len) { + ifc->ifc_len = lastlen; + } ifc->ifc_buf = HeapAlloc(GetProcessHeap(), 0, ifc->ifc_len); ioctlRet = ioctl(fd, SIOCGIFCONF, ifc); } while ((ioctlRet == 0) && (ifc->ifc_len != lastlen));
Sorry I can't test this with the application I was using any more (I'd have to debug some other thing that has gone wrong in the last year first) but I can easily test out whatever change is decided upon with my little standalone tester.
-
wine-bugs＠winehq.org