Re: [PATCH] EVMS core 3/4: evms_ioctl.h

Andi Kleen
2002-10-03T15:17:04Z

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