[alsa-devel] [PATCH 06/39] ALSA: seq: obsolete address mode in compatibility layer

[Takashi Iwai]

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