All of lore.kernel.org
 help / color / mirror / Atom feed
* EVIOCSFF macro inconsistency
@ 2014-09-08 18:14 Elias Vanderstuyft
  2014-09-08 18:31 ` Dmitry Torokhov
  0 siblings, 1 reply; 5+ messages in thread
From: Elias Vanderstuyft @ 2014-09-08 18:14 UTC (permalink / raw)
  To: open list:HID CORE LAYER; +Cc: Dmitry Torokhov

Hi everyone,

After inspecting the <linux/input.h> header file, I found that there
is one single ioctl value macro that is inconsistent w.r.t. the other
macros that do an IOC_WRITE :
    EVIOCSFF   _IOC(_IOC_WRITE, 'E', 0x80, sizeof(struct ff_effect))
Why not define it as follows? :
    EVIOCSFF   _IOW('E', 0x80, struct ff_effect)
Apart from having a more readable definition, it also explicitly
reveals type info ("struct ff_effect").
Is it worth to create a patch to fix it?

Thanks,
Elias

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: EVIOCSFF macro inconsistency
  2014-09-08 18:14 EVIOCSFF macro inconsistency Elias Vanderstuyft
@ 2014-09-08 18:31 ` Dmitry Torokhov
  2014-09-08 19:03   ` Elias Vanderstuyft
  0 siblings, 1 reply; 5+ messages in thread
From: Dmitry Torokhov @ 2014-09-08 18:31 UTC (permalink / raw)
  To: Elias Vanderstuyft; +Cc: open list:HID CORE LAYER

Hi ELias,

On Mon, Sep 08, 2014 at 08:14:13PM +0200, Elias Vanderstuyft wrote:
> Hi everyone,
> 
> After inspecting the <linux/input.h> header file, I found that there
> is one single ioctl value macro that is inconsistent w.r.t. the other
> macros that do an IOC_WRITE :
>     EVIOCSFF   _IOC(_IOC_WRITE, 'E', 0x80, sizeof(struct ff_effect))
> Why not define it as follows? :
>     EVIOCSFF   _IOW('E', 0x80, struct ff_effect)
> Apart from having a more readable definition, it also explicitly
> reveals type info ("struct ff_effect").

I think it is just historical.

> Is it worth to create a patch to fix it?

Sure, why not.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: EVIOCSFF macro inconsistency
  2014-09-08 18:31 ` Dmitry Torokhov
@ 2014-09-08 19:03   ` Elias Vanderstuyft
  2014-09-08 20:24     ` Dmitry Torokhov
  0 siblings, 1 reply; 5+ messages in thread
From: Elias Vanderstuyft @ 2014-09-08 19:03 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: open list:HID CORE LAYER

On Mon, Sep 8, 2014 at 8:31 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi Elias,
>
> On Mon, Sep 08, 2014 at 08:14:13PM +0200, Elias Vanderstuyft wrote:
>> Hi everyone,
>>
>> After inspecting the <linux/input.h> header file, I found that there
>> is one single ioctl value macro that is inconsistent w.r.t. the other
>> macros that do an IOC_WRITE :
>>     EVIOCSFF   _IOC(_IOC_WRITE, 'E', 0x80, sizeof(struct ff_effect))
>> Why not define it as follows? :
>>     EVIOCSFF   _IOW('E', 0x80, struct ff_effect)
>> Apart from having a more readable definition, it also explicitly
>> reveals type info ("struct ff_effect").
>
> I think it is just historical.
>
>> Is it worth to create a patch to fix it?
>
> Sure, why not.

Cool, thanks!

So probably the same applies to the IOC_READ counter parts? :
    EVIOCGBIT(ev,len)    _IOC(_IOC_READ, 'E', 0x20 + (ev), len)
    EVIOCGKEY(len)    _IOC(_IOC_READ, 'E', 0x18, len)
    EVIOCGLED(len)    _IOC(_IOC_READ, 'E', 0x19, len)
    EVIOCGMTSLOTS(len)    _IOC(_IOC_READ, 'E', 0x0a, len)
    EVIOCGNAME(len)    _IOC(_IOC_READ, 'E', 0x06, len)
    EVIOCGPHYS(len)    _IOC(_IOC_READ, 'E', 0x07, len)
    EVIOCGPROP(len)    _IOC(_IOC_READ, 'E', 0x09, len)
    EVIOCGSND(len)    _IOC(_IOC_READ, 'E', 0x1a, len)
    EVIOCGSW(len)    _IOC(_IOC_READ, 'E', 0x1b, len)
    EVIOCGUNIQ(len)    _IOC(_IOC_READ, 'E', 0x08, len)
to be converted to:
    EVIOCGBIT(ev,len)    _IOR('E', 0x20 + (ev), len)
    EVIOCGKEY(len)    _IOR('E', 0x18, len)
    EVIOCGLED(len)    _IOR('E', 0x19, len)
    EVIOCGMTSLOTS(len)    _IOR('E', 0x0a, len)
    EVIOCGNAME(len)    _IOR('E', 0x06, len)
    EVIOCGPHYS(len)    _IOR('E', 0x07, len)
    EVIOCGPROP(len)    _IOR('E', 0x09, len)
    EVIOCGSND(len)    _IOR('E', 0x1a, len)
    EVIOCGSW(len)    _IOR('E', 0x1b, len)
    EVIOCGUNIQ(len)    _IOR('E', 0x08, len)

Thanks,
Elias

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: EVIOCSFF macro inconsistency
  2014-09-08 19:03   ` Elias Vanderstuyft
@ 2014-09-08 20:24     ` Dmitry Torokhov
  2014-09-08 20:42       ` Elias Vanderstuyft
  0 siblings, 1 reply; 5+ messages in thread
From: Dmitry Torokhov @ 2014-09-08 20:24 UTC (permalink / raw)
  To: Elias Vanderstuyft; +Cc: open list:HID CORE LAYER

On Monday, September 08, 2014 09:03:11 PM Elias Vanderstuyft wrote:
> On Mon, Sep 8, 2014 at 8:31 PM, Dmitry Torokhov
> 
> <dmitry.torokhov@gmail.com> wrote:
> > Hi Elias,
> > 
> > On Mon, Sep 08, 2014 at 08:14:13PM +0200, Elias Vanderstuyft wrote:
> >> Hi everyone,
> >> 
> >> After inspecting the <linux/input.h> header file, I found that there
> >> is one single ioctl value macro that is inconsistent w.r.t. the other
> >> 
> >> macros that do an IOC_WRITE :
> >>     EVIOCSFF   _IOC(_IOC_WRITE, 'E', 0x80, sizeof(struct ff_effect))
> >> 
> >> Why not define it as follows? :
> >>     EVIOCSFF   _IOW('E', 0x80, struct ff_effect)
> >> 
> >> Apart from having a more readable definition, it also explicitly
> >> reveals type info ("struct ff_effect").
> > 
> > I think it is just historical.
> > 
> >> Is it worth to create a patch to fix it?
> > 
> > Sure, why not.
> 
> Cool, thanks!
> 
> So probably the same applies to the IOC_READ counter parts? :
>     EVIOCGBIT(ev,len)    _IOC(_IOC_READ, 'E', 0x20 + (ev), len)
>     EVIOCGKEY(len)    _IOC(_IOC_READ, 'E', 0x18, len)
>     EVIOCGLED(len)    _IOC(_IOC_READ, 'E', 0x19, len)
>     EVIOCGMTSLOTS(len)    _IOC(_IOC_READ, 'E', 0x0a, len)
>     EVIOCGNAME(len)    _IOC(_IOC_READ, 'E', 0x06, len)
>     EVIOCGPHYS(len)    _IOC(_IOC_READ, 'E', 0x07, len)
>     EVIOCGPROP(len)    _IOC(_IOC_READ, 'E', 0x09, len)
>     EVIOCGSND(len)    _IOC(_IOC_READ, 'E', 0x1a, len)
>     EVIOCGSW(len)    _IOC(_IOC_READ, 'E', 0x1b, len)
>     EVIOCGUNIQ(len)    _IOC(_IOC_READ, 'E', 0x08, len)
> to be converted to:
>     EVIOCGBIT(ev,len)    _IOR('E', 0x20 + (ev), len)
>     EVIOCGKEY(len)    _IOR('E', 0x18, len)
>     EVIOCGLED(len)    _IOR('E', 0x19, len)
>     EVIOCGMTSLOTS(len)    _IOR('E', 0x0a, len)
>     EVIOCGNAME(len)    _IOR('E', 0x06, len)
>     EVIOCGPHYS(len)    _IOR('E', 0x07, len)
>     EVIOCGPROP(len)    _IOR('E', 0x09, len)
>     EVIOCGSND(len)    _IOR('E', 0x1a, len)
>     EVIOCGSW(len)    _IOR('E', 0x1b, len)
>     EVIOCGUNIQ(len)    _IOR('E', 0x08, len)

No, because 'len' is not a type.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: EVIOCSFF macro inconsistency
  2014-09-08 20:24     ` Dmitry Torokhov
@ 2014-09-08 20:42       ` Elias Vanderstuyft
  0 siblings, 0 replies; 5+ messages in thread
From: Elias Vanderstuyft @ 2014-09-08 20:42 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: open list:HID CORE LAYER

On Mon, Sep 8, 2014 at 10:24 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Monday, September 08, 2014 09:03:11 PM Elias Vanderstuyft wrote:
>> On Mon, Sep 8, 2014 at 8:31 PM, Dmitry Torokhov
>>
>> <dmitry.torokhov@gmail.com> wrote:
>> > Hi Elias,
>> >
>> > On Mon, Sep 08, 2014 at 08:14:13PM +0200, Elias Vanderstuyft wrote:
>> >> Hi everyone,
>> >>
>> >> After inspecting the <linux/input.h> header file, I found that there
>> >> is one single ioctl value macro that is inconsistent w.r.t. the other
>> >>
>> >> macros that do an IOC_WRITE :
>> >>     EVIOCSFF   _IOC(_IOC_WRITE, 'E', 0x80, sizeof(struct ff_effect))
>> >>
>> >> Why not define it as follows? :
>> >>     EVIOCSFF   _IOW('E', 0x80, struct ff_effect)
>> >>
>> >> Apart from having a more readable definition, it also explicitly
>> >> reveals type info ("struct ff_effect").
>> >
>> > I think it is just historical.
>> >
>> >> Is it worth to create a patch to fix it?
>> >
>> > Sure, why not.
>>
>> Cool, thanks!
>>
>> So probably the same applies to the IOC_READ counter parts? :
>>     EVIOCGBIT(ev,len)    _IOC(_IOC_READ, 'E', 0x20 + (ev), len)
>>     EVIOCGKEY(len)    _IOC(_IOC_READ, 'E', 0x18, len)
>>     EVIOCGLED(len)    _IOC(_IOC_READ, 'E', 0x19, len)
>>     EVIOCGMTSLOTS(len)    _IOC(_IOC_READ, 'E', 0x0a, len)
>>     EVIOCGNAME(len)    _IOC(_IOC_READ, 'E', 0x06, len)
>>     EVIOCGPHYS(len)    _IOC(_IOC_READ, 'E', 0x07, len)
>>     EVIOCGPROP(len)    _IOC(_IOC_READ, 'E', 0x09, len)
>>     EVIOCGSND(len)    _IOC(_IOC_READ, 'E', 0x1a, len)
>>     EVIOCGSW(len)    _IOC(_IOC_READ, 'E', 0x1b, len)
>>     EVIOCGUNIQ(len)    _IOC(_IOC_READ, 'E', 0x08, len)
>> to be converted to:
>>     EVIOCGBIT(ev,len)    _IOR('E', 0x20 + (ev), len)
>>     EVIOCGKEY(len)    _IOR('E', 0x18, len)
>>     EVIOCGLED(len)    _IOR('E', 0x19, len)
>>     EVIOCGMTSLOTS(len)    _IOR('E', 0x0a, len)
>>     EVIOCGNAME(len)    _IOR('E', 0x06, len)
>>     EVIOCGPHYS(len)    _IOR('E', 0x07, len)
>>     EVIOCGPROP(len)    _IOR('E', 0x09, len)
>>     EVIOCGSND(len)    _IOR('E', 0x1a, len)
>>     EVIOCGSW(len)    _IOR('E', 0x1b, len)
>>     EVIOCGUNIQ(len)    _IOR('E', 0x08, len)
>
> No, because 'len' is not a type.

Indeed, just found out by myself, thank you for the confirmation.

Elias

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-09-08 20:42 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-08 18:14 EVIOCSFF macro inconsistency Elias Vanderstuyft
2014-09-08 18:31 ` Dmitry Torokhov
2014-09-08 19:03   ` Elias Vanderstuyft
2014-09-08 20:24     ` Dmitry Torokhov
2014-09-08 20:42       ` Elias Vanderstuyft

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.