To: Kevin Corry <corryk@us.ibm.com>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH] EVMS core 3/4: evms_ioctl.h
References: <02100307370503.05904@boiler.suse.lists.linux.kernel>
From: Andi Kleen <ak@suse.de>
Date: 03 Oct 2002 17:22:03 +0200
In-Reply-To: Kevin Corry's message of "3 Oct 2002 15:22:49 +0200"
Message-ID: <p73vg4jr1ic.fsf@oldwotan.suse.de>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1916
Lines: 62

Kevin Corry <corryk@us.ibm.com> writes:


> +struct evms_plugin_ioctl_pkt {
> +	ulong feature_id;
> +	s32 feature_command;
> +	s32 status;
> +	void *feature_ioctl_data;
> +};

This is passed between user space and kernel space right? 

For 32bit emulation on 64bit purposes you should always use explicitely
sized types (u32/u64 not ulong). The pointer will still need to be 
converted. Best is to avoid pointers if possible (e.g. couldn't the data
just be tacked on here?) 


> +#define EVMS_EVENT_END_OF_DISCOVERY     0
> +
> +/**
> + * struct evms_notify_pkt - evms event notification ioctl packet definition
> + * @command:	0 = unregister, 1 = register
> + * @eventry:	event structure
> + * @status:	returned operation status
> + *
> + * ioctl packet definition for EVMS_PROCESS_NOTIFY_EVENT ioctl
> + **/
> +struct evms_notify_pkt {
> +	s32 command;
> +	struct evms_event eventry;

If eventry contains any potential 64bit stuff it would be best to align it 
to 64bit explicitely
> + **/
> +struct evms_user_disk_info_pkt {
> +	u32 status;
> +	u32 flags;
> +	u64 disk_handle;
> +	u32 disk_dev;
> +	u32 geo_sectors;
> +	u32 geo_heads;
> +	u64 geo_cylinders;

emulation trap: on x86-64/ia64 u64 have different alignment on 32bit vs
64bit (4 bytes vs natural). Please make sure that u64 is always explicitely
64bit aligned. It isn't here.


> +	u64 disk_handle;
> +	s32 io_flag;
> +	u64 starting_sector;

Same issue


It would be best to clean the ABI up now when you can still change it.
Otherwise the emulation functions later will be very ugly
(take a look at the LVM horror in arch/x86_64/ia32/ia32_ioctl.c for a 
bad example - LVM1 wasn't cleaned up in time)

-Andi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/