why not choose another way to define the _IOC_xxxMASK related to the ioctl
it defines in the kernel: #define _IOC_NRMASK ((1 << _IOC_NRBITS)-1) //define ... #define _IOC_NRSHIFT 0 ... #define _IOC_DIR(nr) (((nr) >> _IOC_DIRSHIFT) & _IOC_DIRMASK) //when decode why not define it like this: #define _IOC_NRSHIFT 0 ... #define _IOC_NRMASK ((_IOC_NRSHIFT >> _IOC_NRBITS) - _IOC_NRSHIFT) //define ... #define _IOC_DIR(nr) ((nr & _IOC_DIRMASK) >> _IOC_DIRSHIFT) // when decode I think it is better for the word "mask" .
On Sat, 30 Mar 2013, RS wrote:
it defines in the kernel: #define _IOC_NRMASK ((1 << _IOC_NRBITS)-1) //define ... #define _IOC_NRSHIFT 0 ... #define _IOC_DIR(nr) (((nr) >> _IOC_DIRSHIFT) & _IOC_DIRMASK) //when decode
why not define it like this: #define _IOC_NRSHIFT 0 ... #define _IOC_NRMASK ((_IOC_NRSHIFT >> _IOC_NRBITS) - _IOC_NRSHIFT) //define ... #define _IOC_DIR(nr) ((nr & _IOC_DIRMASK) >> _IOC_DIRSHIFT) // when decode
I think it is better for the word "mask" .
It can't be better this way because it's wrong. Let's compute a bit: You are referring include/uapi/asm-generic/ioctl.h AFAICS: #define _IOC_NRBITS 8 #define _IOC_NRSHIFT 0 #define _IOC_NRMASK ((1 << _IOC_NRBITS)-1) This gives _IOC_NRMASK = ((1 << 8) - 1) = 0xff. But supposing your _IOC_NRMASK ((_IOC_NRSHIFT >> _IOC_NRBITS) - _IOC_NRSHIFT) it yields _IOC_NRMASK = ((0 >> 8) - 0) = 0x00. You see? With your mask you'll never get any bits out of the ioctl number because it's just wrong. The same applies to _IOC_DIR(nr). You can't just exchange bitwise ands and shifts - you'd have to change the constants for this... and there is really no point in it. Regards, Tobi
Think you ! Oh , I was wrong . How careless I was. I have fixed it : #define _IOC_NRMASK (((1 << _IOC_NRBITS)-1) << _IOC_NRSHIFT) // if the _IOC_NRBITS is 8 ,then the result is 0XFF #define _IOC_DIR(nr) ((nr & _IOC_DIRMASK) >> _IOC_DIRSHIFT) // when decode , keep the same. Now I think this will spend more time than the kernel code when executed. It performs an extra operation ">>", but I think it better to understand. _IOC_DIRMASK 0xC0000000 _IOC_TYPEMASK 0x0000FF00 _IOC_NRMASK 0x000000FF _IOC_SIZEMASK 0x3FFF0000 At 2013-03-30 16:41:55,"Tobias Boege" <tobias@gambas-buch.de> wrote:
On Sat, 30 Mar 2013, RS wrote:
it defines in the kernel: #define _IOC_NRMASK ((1 << _IOC_NRBITS)-1) //define ... #define _IOC_NRSHIFT 0 ... #define _IOC_DIR(nr) (((nr) >> _IOC_DIRSHIFT) & _IOC_DIRMASK) //when decode
why not define it like this: #define _IOC_NRSHIFT 0 ... #define _IOC_NRMASK ((_IOC_NRSHIFT >> _IOC_NRBITS) - _IOC_NRSHIFT) //define ... #define _IOC_DIR(nr) ((nr & _IOC_DIRMASK) >> _IOC_DIRSHIFT) // when decode
I think it is better for the word "mask" .
It can't be better this way because it's wrong. Let's compute a bit:
You are referring include/uapi/asm-generic/ioctl.h AFAICS:
#define _IOC_NRBITS 8 #define _IOC_NRSHIFT 0 #define _IOC_NRMASK ((1 << _IOC_NRBITS)-1)
This gives _IOC_NRMASK = ((1 << 8) - 1) = 0xff. But supposing your
_IOC_NRMASK ((_IOC_NRSHIFT >> _IOC_NRBITS) - _IOC_NRSHIFT)
it yields _IOC_NRMASK = ((0 >> 8) - 0) = 0x00.
You see? With your mask you'll never get any bits out of the ioctl number because it's just wrong.
The same applies to _IOC_DIR(nr). You can't just exchange bitwise ands and shifts - you'd have to change the constants for this... and there is really no point in it.
Regards, Tobi
_______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
On Sat, 30 Mar 2013 18:01:32 +0800, RS said:
Now I think this will spend more time than the kernel code when executed.
Have you actually examined the generated code on several popular architectures to see what gcc actually does? (hint - many things can constant-folded at compile time. So if the 3 values are #defined to constants, the expression (_IOC_NRSHIFT >> _IOC_NRBITS) - _IOC_NRSHIFT) will generate no actual shift or subtract instructions, merely another constant.
participants (3)
-
RS -
Tobias Boege -
Valdis.Kletnieks＠vt.edu