[alsa-devel] [PATCH] Provide card number / PID via sequencer client info

[Takashi Iwai]

On Sat, 13 Feb 2016 14:42:17 +0100,
Martin Koegler wrote:
> 
> From: Martin Koegler <martin.koegler at chello.at>
> 
> rawmidi devices expose the card number via IOCTLs, which allows to
> find the corresponding device in sysfs.
> 
> The sequencer provides no identifing data. Chromium works around this
> issue by scanning rawmidi as well as sequencer devices and matching
> them by using assumtions, how the kernel register sequencer devices.
> 
> This changes adds support for exposing the card number for kernel clients
> as well as the PID for user client.
> 
> The minor of the API version is changed to distinguish between the zero
> initialised reserved field and card number 0.
> 
> Signed-off-by: Martin Koegler <martin.koegler at chello.at>

The idea looks good.  One remaining question is whether only providing
the card number or pid is sufficient for all cases.

Some review comments below:

> ---
>  include/uapi/sound/asequencer.h |  5 +++--
>  sound/core/seq/seq_clientmgr.c  | 18 ++++++++++++++++++
>  sound/core/seq/seq_clientmgr.h  |  2 ++
>  3 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/include/uapi/sound/asequencer.h b/include/uapi/sound/asequencer.h
> index 5a5fa49..4726579 100644
> --- a/include/uapi/sound/asequencer.h
> +++ b/include/uapi/sound/asequencer.h
> @@ -25,7 +25,7 @@
>  #include <sound/asound.h>
>  
>  /** version of the sequencer */
> -#define SNDRV_SEQ_VERSION SNDRV_PROTOCOL_VERSION (1, 0, 1)
> +#define SNDRV_SEQ_VERSION SNDRV_PROTOCOL_VERSION (1, 0, 2)
>  
>  /**
>   * definition of sequencer event types
> @@ -357,7 +357,8 @@ struct snd_seq_client_info {
>  	unsigned char event_filter[32];	/* event filter bitmap */
>  	int num_ports;			/* RO: number of ports */
>  	int event_lost;			/* number of lost events */
> -	char reserved[64];		/* for future use */
> +	int owner;			/* RO: card number[kernel] / PID[user] */
> +	char reserved[64 - sizeof(int)]; /* for future use */

The sizeof(int) is always 4.  So let's make it explicit.

But, please make sure that this change won't lead to any
incompatibility.  Some architectures might align with 64bit long,
although the 32bit int and the rest char[] should be fine on all known
architectures, AFAIK.


>  };
>  
>  
> diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
> index 58e79e0..ac07ab7 100644
> --- a/sound/core/seq/seq_clientmgr.c
> +++ b/sound/core/seq/seq_clientmgr.c
> @@ -364,6 +364,7 @@ static int snd_seq_open(struct inode *inode, struct file *file)
>  	/* fill client data */
>  	user->file = file;
>  	sprintf(client->name, "Client-%d", c);
> +	client->data.user.owner = get_pid(task_pid(current));
>  
>  	/* make others aware this new client */
>  	snd_seq_system_client_ev_client_start(c);
> @@ -380,6 +381,7 @@ static int snd_seq_release(struct inode *inode, struct file *file)
>  		seq_free_client(client);
>  		if (client->data.user.fifo)
>  			snd_seq_fifo_delete(&client->data.user.fifo);
> +		put_pid(client->data.user.owner);
>  		kfree(client);
>  	}

Shouldn't the counterpart (put_pid()) be called anywhere else?


thanks,

Takashi