All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Henrik Rydberg" <rydberg@euromail.se>
To: Chase Douglas <chasedouglas@gmail.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] Input: Add EVIOC mechanism for MT slots
Date: Sun, 5 Feb 2012 20:40:36 +0100	[thread overview]
Message-ID: <20120205194036.GA2566@polaris.bitmath.org> (raw)
In-Reply-To: <4F2EB8C8.9090102@gmail.com>

> > Besides leaving a possible giant stack crash in your code, it assumes
> > memory is somehow magically allocated. Not good practise in low-level
> > programming. You wouldn't use a template this way, would you?
> 
> No, which is why I think this interface is bad. I should be able to
> dynamically set the size of the array, but it's not possible with this
> interface.

It is possible (using num_slots == 0 or creating your own struct), but
not ideal, granted. The patch serves the purpose of definining the
binary interface, the rest is up to userland.

> I think the implementation is fine in terms of how the plumbing works. I
> just don't think this macro should be included. Make the user create the
> struct themselves:
> 
> In linux/input.h:
> 
> struct input_mt_request {
> 	__u32 code;
> 	__s32 values[];
> };

The above (the first) version is not ideal either, since it cannot be
used as it is.

> It could be argued that we should leave the macro around for people who
> want to statically define the size of the request, but I think that is
> leading them down the wrong path. It's easier, but it will lead to
> broken code if you pick the wrong size.

Rather than creating both a suboptimal static and a suboptimal dynamic
version, removing the struct altogether is tempting. All that is
really needed is a clear definition of the binary interface. The rest
can easily be set up in userland, using whatever method is preferred.

Thanks.
Henrik

  reply	other threads:[~2012-02-05 19:39 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-31 19:00 [PATCH v3] Input: Add EVIOC mechanism for MT slots Henrik Rydberg
2012-02-03  0:05 ` Chase Douglas
2012-02-03  7:27   ` Henrik Rydberg
2012-02-04 18:21     ` Chase Douglas
2012-02-05  7:59       ` Henrik Rydberg
2012-02-05 17:13         ` Chase Douglas
2012-02-05 19:40           ` Henrik Rydberg [this message]
2012-02-05 22:55             ` Chase Douglas
2012-02-06  7:25               ` Dmitry Torokhov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120205194036.GA2566@polaris.bitmath.org \
    --to=rydberg@euromail.se \
    --cc=chasedouglas@gmail.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.