On Mon, Feb 15, 2016 at 11:30:15AM +0100, Takashi Iwai wrote:
> The idea looks good. One remaining question is whether only providing
> the card number or pid is sufficient for all cases.
I only care about the kernel client.
> > @@ -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.
Sorry, I don't have access to the various non x86-architectures, so I can't test.
I will change to
int card;
int pid;
char reserve[56];
More safety would just provide a much more complex (ugly?) structure:
struct _int_snd_seq_client_info_old {
int client; /* client number to inquire */
snd_seq_client_type_t type; /* client type */
char name[64]; /* client name */
unsigned int filter; /* filter flags */
unsigned char multicast_filter[8]; /* multicast filter bitmap */
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 */
};
struct _int_snd_seq_client_info_new {
int client; /* client number to inquire */
snd_seq_client_type_t type; /* client type */
char name[64]; /* client name */
unsigned int filter; /* filter flags */
unsigned char multicast_filter[8]; /* multicast filter bitmap */
unsigned char event_filter[32]; /* event filter bitmap */
int num_ports; /* RO: number of ports */
int event_lost; /* number of lost events */
int card;
int pid;
char reserved[64]; /* for future use */
};
struct snd_seq_client_info {
int client; /* client number to inquire */
snd_seq_client_type_t type; /* client type */
char name[64]; /* client name */
unsigned int filter; /* filter flags */
unsigned char multicast_filter[8]; /* multicast filter bitmap */
unsigned char event_filter[32]; /* event filter bitmap */
int num_ports; /* RO: number of ports */
int event_lost; /* number of lost events */
int card;
int pid;
char reserved[64 - sizeof(struct _int_snd_seq_client_info_new) + sizeof(_int_snd_seq_client_info_old)]; /* for future use */
};
This should handle all alignment and type size issues automatically.
If you really want this, I can provide the apropriate patches.
> >
> > 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?
The only way to free a client structure is seq_free_client1 (static).
This is called by snd_seq_open before get_pid [=> irelevant] and
seq_free_client (static).
seq_free_client is called by snd_seq_release, which calls put_pid and
snd_seq_delete_kernel_client, which destroys a kernel client.
Are I'm missing a code path?
Regards,
Martin
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
