* why not choose another way to define the _IOC_xxxMASK related to the ioctl
@ 2013-03-30 5:47 RS
2013-03-30 8:41 ` Tobias Boege
0 siblings, 1 reply; 4+ messages in thread
From: RS @ 2013-03-30 5:47 UTC (permalink / raw)
To: kernelnewbies
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" .
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20130330/d0df0713/attachment.html
^ permalink raw reply [flat|nested] 4+ messages in thread
* why not choose another way to define the _IOC_xxxMASK related to the ioctl
2013-03-30 5:47 why not choose another way to define the _IOC_xxxMASK related to the ioctl RS
@ 2013-03-30 8:41 ` Tobias Boege
2013-03-30 10:01 ` RS
0 siblings, 1 reply; 4+ messages in thread
From: Tobias Boege @ 2013-03-30 8:41 UTC (permalink / raw)
To: kernelnewbies
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* why not choose another way to define the _IOC_xxxMASK related to the ioctl
2013-03-30 8:41 ` Tobias Boege
@ 2013-03-30 10:01 ` RS
2013-03-30 15:59 ` Valdis.Kletnieks at vt.edu
0 siblings, 1 reply; 4+ messages in thread
From: RS @ 2013-03-30 10:01 UTC (permalink / raw)
To: kernelnewbies
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 at kernelnewbies.org
>http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20130330/8c1f7b89/attachment.html
^ permalink raw reply [flat|nested] 4+ messages in thread* why not choose another way to define the _IOC_xxxMASK related to the ioctl
2013-03-30 10:01 ` RS
@ 2013-03-30 15:59 ` Valdis.Kletnieks at vt.edu
0 siblings, 0 replies; 4+ messages in thread
From: Valdis.Kletnieks at vt.edu @ 2013-03-30 15:59 UTC (permalink / raw)
To: 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 865 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20130330/ba725e47/attachment.bin
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-03-30 15:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-30 5:47 why not choose another way to define the _IOC_xxxMASK related to the ioctl RS
2013-03-30 8:41 ` Tobias Boege
2013-03-30 10:01 ` RS
2013-03-30 15:59 ` Valdis.Kletnieks at vt.edu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.