why not choose another way to define the _IOC_xxxMASK related to the ioctl

[Tobias Boege]

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