On 07/12/2016 01:35 PM, Peter Maydell wrote: > I tested this patch with a compile on OSX, and it does compile > without warnings or errors. (NB: haven't tested that it > fixes the warning that was being complained about in the > other patchset.)
Ultimately, the combination of this patch plus the block patchset in question is where we prove if it makes a positive difference, so if you do have a chance to test it on OSX, that would be nice. And that's true whether we keep this version of the patch (with #if __APPLE__) or do a second version based on a witness set at configure time, because the interaction you're testing is if the replacement #define SIZE_MAX silences the warning about printf(%zd). > > I don't have a very strong opinion about whether it's the > best fix, but a couple of thoughts: > * my inclination is to prefer not to override system > headers except where we've checked and know they're broken > (ie in a future world where Apple get their headers right > I'd rather we automatically ended up using their version > rather than ours) Indeed, a configure-based solution would avoid checkpatch.pl griping about adding an #if __APPLE__. And since glibc has the same bug with SSIZE_MAX, a configure-based solution would be easier to copy-and-paste if we needed to work around that too (then again, we don't have any use of SSIZE_MAX at the moment). > * we don't have any #if ...SIZE_MAX, but we do have some > for other kinds of _MAX constant. I'll wait a bit longer to see if any other opinions surface, but sounds like I'll probably be doing a v2 and trying for a configure solution. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
