[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] ioctl() numbers are unsigned (the man page lies
From: |
Jan Kiszka |
Subject: |
Re: [Qemu-devel] [PATCH] ioctl() numbers are unsigned (the man page lies) |
Date: |
Thu, 09 Feb 2012 09:30:09 +0100 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 |
Looks like the subject would warmly welcome a "kvm:" tag.
On 2012-02-09 05:22, 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.
>
> Signed-off-by: David Gibson <address@hidden>
> ---
> kvm-all.c | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/kvm-all.c b/kvm-all.c
> index 0b87658..681ad15 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -78,7 +78,10 @@ struct KVMState
> int pit_in_kernel;
> int xsave, xcrs;
> int many_ioeventfds;
> - int irqchip_inject_ioctl;
> + /* The man page (and posix) say ioctl numbers are signed int, but
> + * they're not. Linux, glibc and *BSD all treat ioctl numbers as
> + * unsigned, and treating them as signed here can break things */
What about naming the problem instead:
/* Comparison with IOCTL macros on 32-bit hosts requires unsigned. */
> + unsigned irqchip_inject_ioctl;
> #ifdef KVM_CAP_IRQ_ROUTING
> struct kvm_irq_routing *irq_routes;
> int nr_allocated_irq_routes;
Jan
signature.asc
Description: OpenPGP digital signature