Re: [lkcd-devel] Changing ioctl cmds to make 32/64 bit clean

[Haren M. <hb...@us...>]

Hello Arnd, 
        Thanks for your response. Please see below for my comments in italic. 


On Saturday 10 January 2004 07:54, Haren Myneni wrote:
 
> But at present, we  used COMMANDS between 1 to 15 which may not be 
unique
> in the system. Hence I am proposing to define these commands using
> _IOR(type, nr, dataitem) and _IOW(type, nr, dataitem).
> Where type is 0x10E6 -  ( MISC_MAJOR number= 10, /dev/dump minor number 
=
> E6(230)
>         nr  is the actual command that is already defined
>         dataitem  - sizeof (ulong)
>
> EX: _IOR(0x10E6, DIOSDUMPDEV, sizeof(ulong))
 
Well, that won't work as you expect it to, although it is a step in the 
right
direction. First, the type argument is only 8 bit, so in your example, 
0x10E6
would be identical to 0xE6.
More importantly, if you are making incompatible changes to the ABI, at 
least
do them in the way that they are transparent to 32 bit emulation. Always 
pass
__u32, __u64 or similar data as ioctl arguments (int and short are ok if 
already
established), but _never_ size_t, dev_t, long or even pointers.
 
The example should look like:
#define DIOSDUMPDEV _IOW(0xe6, 23, __u64)
 
Also note the correct use of the macro name, using the correct direction 
(_IOW,
not _IOR) and the absence of sizeof (you don't want to know what sizeof 
does
there). 

Somehow it worked with 0x0AE6. Probably it had taken the last 8 bits as 
you said and could not find any conflict on my system. Thanks for letting 
me know as I could not find that the size is 8 bits. If E6 is the major 
number for some driver, then we could get in to conflict. Based on the 
present kernel sources, some drivers used their major number as its magic 
value since it is unique in the system. But some drivers (nvram and 
/dev/rtc) which had MISC_MAJOR number like ours, used 'p' as the magic 
value and some reserved values (00-9F given in 
Documentation/ioctl-number.txt) for 'nr' value. Is it OK if I used A0 to 
B0 instead of 0-16 as we are using now. 

Yes, During testing, I found out that the dataitem should not be 'sizeof', 
should be 'unsigned int' or __u32.

We need both _IOW and _IOR. As you said. _IOW will be used to pass 
configuration values to the driver.  Whereas, _IOR is needed to read the 
present configuration values from the driver. Used for 'lkcd_config -q' 

_IOR('p', A0, __u32) and _IOW('p', A1, __u32)      
 
>   Even though I am planning to modify only for PPC64 arch
> (arch/ppc64/kernel/ioctl32.c),  we should extend these changes to other
> archs later by modifying  fs/compat_ioctl.c file. However, the user 
level
> lkcd_config.c file has to be modified to pass these new commands. The
> present values will be used for 2.4 or before to make the lkcd_config 
work
> for these versions.
 
You can make it more generic if you use the register_ioctl32_conversion()
function to make it architecture independent. For 2.6, just list them in
include/linux/compat_ioctl.h. 

Yes, we could use either register_ioctl32_conversion or list them in 
include/linux/compat_ioctl.h to make them generic. I am not sure, whether 
everyone is OK if I make them generic since only PPC64 implementation 
needs this conversion right now. Once, the LKCD is ported to other 
platforms like x86-64 and IA64, we could move these changes to the generic 
header file. Suggestions are welcome .

Thanks
Haren
 
Arnd <><
 


-------------------------------------------------------
This SF.net email is sponsored by: Perforce Software.
Perforce is the Fast Software Configuration Management System offering
advanced branching capabilities and atomic changes on 50+ platforms.
Free Eval! http://www.perforce.com/perforce/loadprog.html
_______________________________________________
lkcd-devel mailing list
lkc...@li... 
https://lists.sourceforge.net/lists/listinfo/lkcd-devel