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);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