On Tue, 09 Aug 2016 14:04:37 +0200, Takashi Sakamoto wrote:
On Aug 8 2016 23:59, Takashi Iwai wrote:
On Mon, 08 Aug 2016 14:19:50 +0200, Takashi Sakamoto wrote:
On Aug 8 2016 16:09, Takashi Iwai wrote:
It's better to pass a snd_seq_client pointer to this function and evaluate client->port in the function instead of referencing to client->port from each caller.
?
I don't understand to what you addressed. Neither snd_seq_do_ioctl() and snd_seq_kernel_client_ctl() have differences about port referencing.
Put in this way: why did you change seq_call_port_ioctl() to take an clientid integer instead of client pointer? Due to this change, each caller has to deduce the client number by referencing client->number. Which merit does it give at all?
Just to call snd_seq_kernel_client_ctl(). The callers are in 32/64 bit compatibility code and the function is not exported.
I don't realize what you concern yet.
Please read back your patch again. You changed the function argument like:
================================================================ @@ -42,13 +42,11 @@ struct snd_seq_port_info32 { char reserved[59]; /* for future use */ };
-static int seq_call_port_info_ioctl(struct snd_seq_client *client, - unsigned int cmd, +static int seq_call_port_info_ioctl(int clientid, unsigned int cmd, struct snd_seq_port_info32 __user *data32) ================================================================
And this requires the changes in all its callers as below:
================================================================ case SNDRV_SEQ_IOCTL_CREATE_PORT32: - return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_CREATE_PORT, argp); + return seq_call_port_info_ioctl(client->number, + SNDRV_SEQ_IOCTL_CREATE_PORT, argp); case SNDRV_SEQ_IOCTL_DELETE_PORT32: - return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_DELETE_PORT, argp); + return seq_call_port_info_ioctl(client->number, + SNDRV_SEQ_IOCTL_DELETE_PORT, argp); case SNDRV_SEQ_IOCTL_GET_PORT_INFO32: - return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_GET_PORT_INFO, argp); + return seq_call_port_info_ioctl(client->number, + SNDRV_SEQ_IOCTL_GET_PORT_INFO, argp); case SNDRV_SEQ_IOCTL_SET_PORT_INFO32: - return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_SET_PORT_INFO, argp); + return seq_call_port_info_ioctl(client->number, + SNDRV_SEQ_IOCTL_SET_PORT_INFO, argp); case SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT32: - return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT, argp); + return seq_call_port_info_ioctl(client->number, + SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT, argp); ================================================================
But, why do you need to change these changes at all? What you basically need is to change the call inside seq_call_port_info_ioctl() like:
- err = snd_seq_do_ioctl(client, cmd, data); + err = snd_seq_kernel_client_ctl(client->number, cmd, data);
Then the rest can remain intact. Referencing client->number in each caller side just results in inefficient codes, obviously.
Takashi