On Mon, Jan 21, 2013 at 07:35:22AM -0700, Eric Blake wrote: > On 01/21/2013 06:14 AM, Andreas Färber wrote: > >> 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? > > uintptr_t feels more correct - the 3rd (vararg) argument through the > ioctl() syscall is always retrieved using the same size as void*. Actually, sys_ioctl() always retrieve it using "unsigned long", but nothing prevents the arch-specific syscall entry code to from translating something from a different type to "unsigned long" before calling sys_ioctl(). So I guess the only guarantee we have is the Linux ioctl(2) man page, that says: "The third argument is an untyped pointer to memory. It's traditionally char *argp (from the days before void * was valid C), and will be so named for this discussion." That said, I plan to change the code to cast the argument to (void*) in the next version. > > > > > 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? > > My discussion about 'int' vs. 'unsigned long' was in regards to the > second argument KVM_CREATE_VCPU, which your patch does not change > (perhaps my fault for jumping in on a conversation mid-thread without > actually reading your original patch, which I have now done). That is, > KVM_CREATE_VCPU as a constant is always 32 bits (kernel constraint), > widened out to unsigned long when passed to the glibc function (due to > the glibc signature disagreeing with POSIX), then narrowed back down to > 32 bits when forwarded to the kernel syscall. > > Meanwhile, your patch is fixing the third argument from 'int' to a wider > type, which is necessary for passing that value through varargs when the > receiving end will retrieve the same argument via a void* variable. I am confident that "unsigned long" will work properly on all architectures we care about today, but I also don't know if this is documented and guaranteed to work on all architectures. Passing an argument of the documented type (void*) sounds like the right thing to do. -- Eduardo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
