Re: [Qemu-devel] [PATCH 1/4] kvm: Comparison with ioctl number macros needs to be unsigned

[Alexander Graf]

On 08.03.2012, at 01:41, David Gibson wrote:

> In kvm-all.c we store an ioctl cmd number in the irqchip_inject_ioctl field
> of KVMState, which has type 'int'.  This seems to make sense since the
> ioctl() man page says that the cmd parameter has type int.
> 
> However, the kernel treats ioctl numbers as unsigned - sys_ioctl() takes an
> unsigned int, and the macros which generate ioctl numbers expand to
> unsigned expressions.  Furthermore, some ioctls (IOC_READ ioctls on x86
> and IOC_WRITE ioctls on powerpc) have bit 31 set, and so would be negative
> if interpreted as an int. This has the surprising and compile-breaking
> consequence that in kvm_irqchip_set_irq() where we do:
>    return (s->irqchip_inject_ioctl == KVM_IRQ_LINE) ? 1 : event.status;
> We will get a "comparison is always false due to limited range of data
> type" warning from gcc if KVM_IRQ_LINE is one of the bit-31-set ioctls,
> which it is on powerpc.
> 
> So, despite the fact that the man page and posix say ioctl numbers are
> signed, they're actually unsigned.  The kernel uses unsigned, the glibc
> header uses unsigned long, and FreeBSD, NetBSD and OSX also use unsigned
> long ioctl numbers in the code.
> 
> Therefore, this patch changes the variable to be unsigned, fixing the
> compile.
> 
> Cc: Avi Kivity <address@hidden>
> Cc: Marcelo Tossatti <address@hidden>
> 
> Signed-off-by: David Gibson <address@hidden>

I picked that one into ppc-next, as without compilation on ppc breaks for me. I 
haven't touched the other 3 patches in the set though.


Alex

PS: These 4 patches don't target a single maintainer. Thus it's better to not 
send them as a patch set :).