-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 18.01.2013 18:46, schrieb Eric Blake: > On 01/18/2013 09:40 AM, Eduardo Habkost wrote: >> On Fri, Jan 18, 2013 at 09:11:42AM -0700, Eric Blake wrote: >>> On 01/18/2013 07:20 AM, Eduardo Habkost wrote: >>>>> Could you suggest a text for me to add please? >>>> >>>> "The argument passed to KVM_CREATE_VCPU now has 'unsigned >>>> long' type instead of 'int', as expected by the Linux ioctl() >>>> syscall. Maybe an int works on most or all architectures >>>> supporting KVM, but it is safer to use an appropriate >>>> 'unsigned long' parameter." >>> >>> Interestingly enough, while the Linux syscall uses 'unsigned >>> long', the POSIX definition of ioctl() uses 'int'; so the Linux >>> kernel is already constrained to never use an ioctl value that >>> doesn't fit within 'int', >> >> Really? What about the ioctl()s that get a pointer as argument >> on architectures where pointers don't fit in an int? >> >> Do you have a pointer to the POSIX definition you are talking >> about? >> >> Note that I'm talking about the the extra ioctl() argument, not >> the ioctl() number (that is an unsigned int in the kernel code). > > Okay, now you made me go back and check sources. > > POSIX 2008 says: #include <stropts.h> int ioctl(int fildes, int > request, ... /* arg */); > > Gnulib says this about a bug that it works around: @item On glibc > platforms, the second parameter is of type @code{unsigned long} > rather than @code{int}. > > But gnulib also suggests using <sys/ioctl.h> instead of the POSIX > header <stropts.h> for getting ioctl(), because <stropts.h> was > declared obsolete in POSIX 2008 and was never implemented in > glibc. > > Sure enough, looking at Fedora 18 /usr/include/sys/ioctl.h, I still > see: extern int ioctl (int __fd, unsigned long int __request, ...) > __THROW; > > Meanwhile, you are correct that the kernel defines request as 32 > bits: linux.git:include/uapi/asm-generic/ioctl.h /* ioctl command > encoding: 32 bits total, command in lower 16 bits, * size of the > parameter structure in the lower 14 bits of the * upper 16 bits. * > Encoding the size of the parameter structure in the ioctl request * > is useful for catching programs compiled with old versions * and to > avoid overwriting user space outside the user buffer area. * The > highest 2 bits are reserved for indicating the ``access mode''. * > NOTE: This limits the max parameter size to 16kB -1 ! */ > >> >>> and glibc is already responsible for ensuring that argument >>> promotion of an int doesn't change the behavior of ioctl() in >>> libc when converting it over to the unsigned long syscall >>> semantics expected by the kernel. > > So a more precise wording of this is: > > glibc is already responsible from converting the 'unsigned long > int' of the user declaration back into the 'unsigned int' that the > kernel expects for the second argument. The third argument (when > present), is generally treated as a pointer (of size appropriate > for the architecture). Although there _might_ be an ioctl that > uses it directly as an integer instead of dereferencing it as a > pointer, those would be the exceptions to the rule.
So ... do we have a conclusion what to put into the commit message? :) It looks to me as if kvm-all.c:kvm_vm_ioctl() is using void*. I like unsigned long but maybe uintptr_t would be more correct then? Or should kvm_vm_ioctl() be fixed to use something else instead? Eric's int would be a semantic change for the 64-bit platforms, no? Andreas - -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (GNU/Linux) iQIcBAEBAgAGBQJQ/T8lAAoJEPou0S0+fgE/gl8QALesZwG5q07W21mp2j4ikL8N jrBHjG2VZ9Kda+AIGMClVsWntGZSOzHdtriJ4gjxp90D71S/LQfsYAy6bj45FIwS kPbIQblLlL5Xc6ZiTS5yTzkwyEd7gUpDVouXyv3XxeyUxqhQKwgWxpiP4RftbBRI Z8wLbVFNpIoIsHfhKoNkT4M/Ucm1iZbChV6y4zqltAfdQhcl6Gq0jzhtkAfmN41t p3tCJYldRwayiKLsO2Y0BMNrKmCJisKCEGmkCQzye/3cuFoat/WUmpjV/65hLNtm ruzfn6pCqMTEGPC4YeDdUsxAhVzVX+Sd4mBKHBGItmvhhJMFYUtwTosRwX5bOrAJ mpVLAj5/XDYTm2/jQUEOJAqpxUr5oAVMQL3sNeWJPmXkk1kNaNWTNVHHDW1iJnRj ty0YIWOnuNabkwiDEjPCz6ghjfA3wOBWy8Gk3+F21MYgRQwDTFw4JZuroOIzy3iD 6Vs4MmiBUGnoLobSqw2dUZFmjL7a1500AxZG0MwBd+EqnbLHGqD33kvLrbUYT8+F eW+cqKV+ZXo3ux343rTxD6EFgmN7GmHSkknxJN5m6ldlw5wfFQ8KhdCiKjwSq3EP X0bVGmryEdIh+6w/RbhL75Vfb/Je0mr/GzhtijtXo+FORkF8ip2mlpVSl46r0AfI KvsZ0HZqZHsfoaSBC1js =/cL3 -----END PGP SIGNATURE-----