[alsa-devel] [PATCH 00/39] seq: obsolete change of address limit
Hi,
ALSA sequencer core has two types of client; user application and kernel driver. The core allows both types of client to do relevant operations, thus it's required for the core to handle data in both user and kernel spaces.
Currently, this is achieved by changing address limit of running task. This is a well-known technique to suppress address check, while it's just a suppression and unfriendly to readers or static code parsers.
This patchset obsoletes the usage. In process context, data in user space is once copied to kernel space, then operated and copied to user space. As a result, actual operations for each ioctl command handle data in kernel space, and '__user' qualifier is useless.
In this series, patch 1-8 just apply above design. The rest changes each operation following to the design. Therefore, static code parser like sparce generates warnings temporarily in a way to apply these patches.
There's a concern of this solution. The data for ioctl is always copied to kernel space or to kernel space even when it's read-only or write-only. But this brings no severe issue as long as I read ALSA sequencer core.
(This patchset is a part of my work to introduce EPIPE into ALSA rawmidi core.)
Takashi Sakamoto (39): ALSA: seq: add const qualifier to table of functions for ioctl ALSA: seq: apply shorter name for file local functions ALSA: seq: fulfill callback entry for ioctl ALSA: seq: copy ioctl data from user space to kernel stack ALSA: seq: add documentation for snd_seq_kernel_client_ctl ALSA: seq: obsolete address mode in compatibility layer ALSA: seq: obsolete change of address limit in in-kernel path for ioctl ALSA: seq: obsolete address limit helper ALSA: seq: optimize pversion function to new design ALSA: seq: optimize client_id function to new design ALSA: seq: optimize system_info function to new design ALSA: seq: optimize running mode function to new design ALSA: seq: optimize client_info function to new design ALSA: seq: optimize set_client_info function to new design ALSA: seq: optimize create_port function to new design ALSA: seq: optimize delete_port function to new design ALSA: seq: optimize get_port_info function to new design ALSA: seq: optimize seq_port_info function to new design ALSA: seq: optimize subscribe_port function to new design ALSA: seq: optimize unsubscribe_port function to new design ALSA: seq: optimize create_queue function to new design ALSA: seq: optimize delete_queue function to new design ALSA: seq: optimize get_queue_info function to new design ALSA: seq: optimize seq_queue_info function to new design ALSA: seq: optimize get_named_queue function to new design ALSA: seq: optimize get_queue_status function to new design ALSA: seq: optimize get_queue_tempo function to new design ALSA: seq: optimize set_queue_tempo function to new design ALSA: seq: optimize get_queue_timer function to new design ALSA: seq: optimize seq_queue_timer function to new design ALSA: seq: optimize get_queue_client function to new design ALSA: seq: optimize set_queue_client function to new design ALSA: seq: optimize get_client_pool function to new design ALSA: seq: optimize seq_client_pool function to new design ALSA: seq: optimize remove_events function to new design ALSA: seq: optimize get_subscription function to new design ALSA: seq: optimize query_subs function to new design ALSA: seq: optimize query_next_client function to new design ALSA: seq: optimize query_next_port function to new design
sound/core/seq/seq_clientmgr.c | 854 +++++++++++++++++++---------------------- sound/core/seq/seq_compat.c | 26 +- 2 files changed, 407 insertions(+), 473 deletions(-)
Each entry in this table is never changed in runtime. This commit moves it from .data to .rodata section.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index d6d9419..42be1e5 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -2168,7 +2168,7 @@ static int snd_seq_ioctl_query_next_port(struct snd_seq_client *client,
/* -------------------------------------------------------- */
-static struct seq_ioctl_table { +static const struct seq_ioctl_table { unsigned int cmd; int (*func)(struct snd_seq_client *client, void __user * arg); } ioctl_tables[] = { @@ -2207,7 +2207,7 @@ static struct seq_ioctl_table { static int snd_seq_do_ioctl(struct snd_seq_client *client, unsigned int cmd, void __user *arg) { - struct seq_ioctl_table *p; + const struct seq_ioctl_table *p;
switch (cmd) { case SNDRV_SEQ_IOCTL_PVERSION:
Exported symbols should have 'snd_' prefix to avoid name conflict, while file local symbols have no need to have. Shorter names are generally preferable due to '80 characters in a line' rule.
This commit renames some local functions.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 180 +++++++++++++++++++++-------------------- sound/core/seq/seq_compat.c | 19 ++--- 2 files changed, 101 insertions(+), 98 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 42be1e5..1331103 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1130,7 +1130,8 @@ static unsigned int snd_seq_poll(struct file *file, poll_table * wait)
/* SYSTEM_INFO ioctl() */ -static int snd_seq_ioctl_system_info(struct snd_seq_client *client, void __user *arg) +static int seq_ioctl_system_info(struct snd_seq_client *client, + void __user *arg) { struct snd_seq_system_info info;
@@ -1150,7 +1151,8 @@ static int snd_seq_ioctl_system_info(struct snd_seq_client *client, void __user
/* RUNNING_MODE ioctl() */ -static int snd_seq_ioctl_running_mode(struct snd_seq_client *client, void __user *arg) +static int seq_ioctl_running_mode(struct snd_seq_client *client, + void __user *arg) { struct snd_seq_running_info info; struct snd_seq_client *cptr; @@ -1213,8 +1215,8 @@ static void get_client_info(struct snd_seq_client *cptr, memset(info->reserved, 0, sizeof(info->reserved)); }
-static int snd_seq_ioctl_get_client_info(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_get_client_info(struct snd_seq_client *client, + void __user *arg) { struct snd_seq_client *cptr; struct snd_seq_client_info client_info; @@ -1237,8 +1239,8 @@ static int snd_seq_ioctl_get_client_info(struct snd_seq_client *client,
/* CLIENT_INFO ioctl() */ -static int snd_seq_ioctl_set_client_info(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_set_client_info(struct snd_seq_client *client, + void __user *arg) { struct snd_seq_client_info client_info;
@@ -1267,8 +1269,8 @@ static int snd_seq_ioctl_set_client_info(struct snd_seq_client *client, /* * CREATE PORT ioctl() */ -static int snd_seq_ioctl_create_port(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_create_port(struct snd_seq_client *client, + void __user *arg) { struct snd_seq_client_port *port; struct snd_seq_port_info info; @@ -1317,8 +1319,8 @@ static int snd_seq_ioctl_create_port(struct snd_seq_client *client, /* * DELETE PORT ioctl() */ -static int snd_seq_ioctl_delete_port(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_delete_port(struct snd_seq_client *client, + void __user *arg) { struct snd_seq_port_info info; int err; @@ -1341,8 +1343,8 @@ static int snd_seq_ioctl_delete_port(struct snd_seq_client *client, /* * GET_PORT_INFO ioctl() (on any client) */ -static int snd_seq_ioctl_get_port_info(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_get_port_info(struct snd_seq_client *client, + void __user *arg) { struct snd_seq_client *cptr; struct snd_seq_client_port *port; @@ -1374,8 +1376,8 @@ static int snd_seq_ioctl_get_port_info(struct snd_seq_client *client, /* * SET_PORT_INFO ioctl() (only ports on this/own client) */ -static int snd_seq_ioctl_set_port_info(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_set_port_info(struct snd_seq_client *client, + void __user *arg) { struct snd_seq_client_port *port; struct snd_seq_port_info info; @@ -1452,8 +1454,8 @@ int snd_seq_client_notify_subscription(int client, int port, /* * add to port's subscription list IOCTL interface */ -static int snd_seq_ioctl_subscribe_port(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_subscribe_port(struct snd_seq_client *client, + void __user *arg) { int result = -EINVAL; struct snd_seq_client *receiver = NULL, *sender = NULL; @@ -1497,8 +1499,8 @@ static int snd_seq_ioctl_subscribe_port(struct snd_seq_client *client, /* * remove from port's subscription list */ -static int snd_seq_ioctl_unsubscribe_port(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_unsubscribe_port(struct snd_seq_client *client, + void __user *arg) { int result = -ENXIO; struct snd_seq_client *receiver = NULL, *sender = NULL; @@ -1539,8 +1541,8 @@ static int snd_seq_ioctl_unsubscribe_port(struct snd_seq_client *client,
/* CREATE_QUEUE ioctl() */ -static int snd_seq_ioctl_create_queue(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_create_queue(struct snd_seq_client *client, + void __user *arg) { struct snd_seq_queue_info info; int result; @@ -1574,8 +1576,8 @@ static int snd_seq_ioctl_create_queue(struct snd_seq_client *client, }
/* DELETE_QUEUE ioctl() */ -static int snd_seq_ioctl_delete_queue(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_delete_queue(struct snd_seq_client *client, + void __user *arg) { struct snd_seq_queue_info info;
@@ -1586,8 +1588,8 @@ static int snd_seq_ioctl_delete_queue(struct snd_seq_client *client, }
/* GET_QUEUE_INFO ioctl() */ -static int snd_seq_ioctl_get_queue_info(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_get_queue_info(struct snd_seq_client *client, + void __user *arg) { struct snd_seq_queue_info info; struct snd_seq_queue *q; @@ -1613,8 +1615,8 @@ static int snd_seq_ioctl_get_queue_info(struct snd_seq_client *client, }
/* SET_QUEUE_INFO ioctl() */ -static int snd_seq_ioctl_set_queue_info(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_set_queue_info(struct snd_seq_client *client, + void __user *arg) { struct snd_seq_queue_info info; struct snd_seq_queue *q; @@ -1649,7 +1651,8 @@ static int snd_seq_ioctl_set_queue_info(struct snd_seq_client *client, }
/* GET_NAMED_QUEUE ioctl() */ -static int snd_seq_ioctl_get_named_queue(struct snd_seq_client *client, void __user *arg) +static int seq_ioctl_get_named_queue(struct snd_seq_client *client, + void __user *arg) { struct snd_seq_queue_info info; struct snd_seq_queue *q; @@ -1672,8 +1675,8 @@ static int snd_seq_ioctl_get_named_queue(struct snd_seq_client *client, void __u }
/* GET_QUEUE_STATUS ioctl() */ -static int snd_seq_ioctl_get_queue_status(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_get_queue_status(struct snd_seq_client *client, + void __user *arg) { struct snd_seq_queue_status status; struct snd_seq_queue *queue; @@ -1706,8 +1709,8 @@ static int snd_seq_ioctl_get_queue_status(struct snd_seq_client *client,
/* GET_QUEUE_TEMPO ioctl() */ -static int snd_seq_ioctl_get_queue_tempo(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_get_queue_tempo(struct snd_seq_client *client, + void __user *arg) { struct snd_seq_queue_tempo tempo; struct snd_seq_queue *queue; @@ -1746,8 +1749,8 @@ int snd_seq_set_queue_tempo(int client, struct snd_seq_queue_tempo *tempo)
EXPORT_SYMBOL(snd_seq_set_queue_tempo);
-static int snd_seq_ioctl_set_queue_tempo(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_set_queue_tempo(struct snd_seq_client *client, + void __user *arg) { int result; struct snd_seq_queue_tempo tempo; @@ -1761,8 +1764,8 @@ static int snd_seq_ioctl_set_queue_tempo(struct snd_seq_client *client,
/* GET_QUEUE_TIMER ioctl() */ -static int snd_seq_ioctl_get_queue_timer(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_get_queue_timer(struct snd_seq_client *client, + void __user *arg) { struct snd_seq_queue_timer timer; struct snd_seq_queue *queue; @@ -1798,8 +1801,8 @@ static int snd_seq_ioctl_get_queue_timer(struct snd_seq_client *client,
/* SET_QUEUE_TIMER ioctl() */ -static int snd_seq_ioctl_set_queue_timer(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_set_queue_timer(struct snd_seq_client *client, + void __user *arg) { int result = 0; struct snd_seq_queue_timer timer; @@ -1840,8 +1843,8 @@ static int snd_seq_ioctl_set_queue_timer(struct snd_seq_client *client,
/* GET_QUEUE_CLIENT ioctl() */ -static int snd_seq_ioctl_get_queue_client(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_get_queue_client(struct snd_seq_client *client, + void __user *arg) { struct snd_seq_queue_client info; int used; @@ -1862,8 +1865,8 @@ static int snd_seq_ioctl_get_queue_client(struct snd_seq_client *client,
/* SET_QUEUE_CLIENT ioctl() */ -static int snd_seq_ioctl_set_queue_client(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_set_queue_client(struct snd_seq_client *client, + void __user *arg) { int err; struct snd_seq_queue_client info; @@ -1877,13 +1880,13 @@ static int snd_seq_ioctl_set_queue_client(struct snd_seq_client *client, return err; }
- return snd_seq_ioctl_get_queue_client(client, arg); + return seq_ioctl_get_queue_client(client, arg); }
/* GET_CLIENT_POOL ioctl() */ -static int snd_seq_ioctl_get_client_pool(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_get_client_pool(struct snd_seq_client *client, + void __user *arg) { struct snd_seq_client_pool info; struct snd_seq_client *cptr; @@ -1917,8 +1920,8 @@ static int snd_seq_ioctl_get_client_pool(struct snd_seq_client *client, }
/* SET_CLIENT_POOL ioctl() */ -static int snd_seq_ioctl_set_client_pool(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_set_client_pool(struct snd_seq_client *client, + void __user *arg) { struct snd_seq_client_pool info; int rc; @@ -1957,13 +1960,13 @@ static int snd_seq_ioctl_set_client_pool(struct snd_seq_client *client, client->pool->room = info.output_room; }
- return snd_seq_ioctl_get_client_pool(client, arg); + return seq_ioctl_get_client_pool(client, arg); }
/* REMOVE_EVENTS ioctl() */ -static int snd_seq_ioctl_remove_events(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_remove_events(struct snd_seq_client *client, + void __user *arg) { struct snd_seq_remove_events info;
@@ -1992,8 +1995,8 @@ static int snd_seq_ioctl_remove_events(struct snd_seq_client *client, /* * get subscription info */ -static int snd_seq_ioctl_get_subscription(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_get_subscription(struct snd_seq_client *client, + void __user *arg) { int result; struct snd_seq_client *sender = NULL; @@ -2032,8 +2035,7 @@ static int snd_seq_ioctl_get_subscription(struct snd_seq_client *client, /* * get subscription info - check only its presence */ -static int snd_seq_ioctl_query_subs(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_query_subs(struct snd_seq_client *client, void __user *arg) { int result = -ENXIO; struct snd_seq_client *cptr = NULL; @@ -2102,8 +2104,8 @@ static int snd_seq_ioctl_query_subs(struct snd_seq_client *client, /* * query next client */ -static int snd_seq_ioctl_query_next_client(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_query_next_client(struct snd_seq_client *client, + void __user *arg) { struct snd_seq_client *cptr = NULL; struct snd_seq_client_info info; @@ -2134,8 +2136,8 @@ static int snd_seq_ioctl_query_next_client(struct snd_seq_client *client, /* * query next port */ -static int snd_seq_ioctl_query_next_port(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_query_next_port(struct snd_seq_client *client, + void __user *arg) { struct snd_seq_client *cptr; struct snd_seq_client_port *port = NULL; @@ -2172,39 +2174,39 @@ static const struct seq_ioctl_table { unsigned int cmd; int (*func)(struct snd_seq_client *client, void __user * arg); } ioctl_tables[] = { - { SNDRV_SEQ_IOCTL_SYSTEM_INFO, snd_seq_ioctl_system_info }, - { SNDRV_SEQ_IOCTL_RUNNING_MODE, snd_seq_ioctl_running_mode }, - { SNDRV_SEQ_IOCTL_GET_CLIENT_INFO, snd_seq_ioctl_get_client_info }, - { SNDRV_SEQ_IOCTL_SET_CLIENT_INFO, snd_seq_ioctl_set_client_info }, - { SNDRV_SEQ_IOCTL_CREATE_PORT, snd_seq_ioctl_create_port }, - { SNDRV_SEQ_IOCTL_DELETE_PORT, snd_seq_ioctl_delete_port }, - { SNDRV_SEQ_IOCTL_GET_PORT_INFO, snd_seq_ioctl_get_port_info }, - { SNDRV_SEQ_IOCTL_SET_PORT_INFO, snd_seq_ioctl_set_port_info }, - { SNDRV_SEQ_IOCTL_SUBSCRIBE_PORT, snd_seq_ioctl_subscribe_port }, - { SNDRV_SEQ_IOCTL_UNSUBSCRIBE_PORT, snd_seq_ioctl_unsubscribe_port }, - { SNDRV_SEQ_IOCTL_CREATE_QUEUE, snd_seq_ioctl_create_queue }, - { SNDRV_SEQ_IOCTL_DELETE_QUEUE, snd_seq_ioctl_delete_queue }, - { SNDRV_SEQ_IOCTL_GET_QUEUE_INFO, snd_seq_ioctl_get_queue_info }, - { SNDRV_SEQ_IOCTL_SET_QUEUE_INFO, snd_seq_ioctl_set_queue_info }, - { SNDRV_SEQ_IOCTL_GET_NAMED_QUEUE, snd_seq_ioctl_get_named_queue }, - { SNDRV_SEQ_IOCTL_GET_QUEUE_STATUS, snd_seq_ioctl_get_queue_status }, - { SNDRV_SEQ_IOCTL_GET_QUEUE_TEMPO, snd_seq_ioctl_get_queue_tempo }, - { SNDRV_SEQ_IOCTL_SET_QUEUE_TEMPO, snd_seq_ioctl_set_queue_tempo }, - { SNDRV_SEQ_IOCTL_GET_QUEUE_TIMER, snd_seq_ioctl_get_queue_timer }, - { SNDRV_SEQ_IOCTL_SET_QUEUE_TIMER, snd_seq_ioctl_set_queue_timer }, - { SNDRV_SEQ_IOCTL_GET_QUEUE_CLIENT, snd_seq_ioctl_get_queue_client }, - { SNDRV_SEQ_IOCTL_SET_QUEUE_CLIENT, snd_seq_ioctl_set_queue_client }, - { SNDRV_SEQ_IOCTL_GET_CLIENT_POOL, snd_seq_ioctl_get_client_pool }, - { SNDRV_SEQ_IOCTL_SET_CLIENT_POOL, snd_seq_ioctl_set_client_pool }, - { SNDRV_SEQ_IOCTL_GET_SUBSCRIPTION, snd_seq_ioctl_get_subscription }, - { SNDRV_SEQ_IOCTL_QUERY_NEXT_CLIENT, snd_seq_ioctl_query_next_client }, - { SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT, snd_seq_ioctl_query_next_port }, - { SNDRV_SEQ_IOCTL_REMOVE_EVENTS, snd_seq_ioctl_remove_events }, - { SNDRV_SEQ_IOCTL_QUERY_SUBS, snd_seq_ioctl_query_subs }, + { SNDRV_SEQ_IOCTL_SYSTEM_INFO, seq_ioctl_system_info }, + { SNDRV_SEQ_IOCTL_RUNNING_MODE, seq_ioctl_running_mode }, + { SNDRV_SEQ_IOCTL_GET_CLIENT_INFO, seq_ioctl_get_client_info }, + { SNDRV_SEQ_IOCTL_SET_CLIENT_INFO, seq_ioctl_set_client_info }, + { SNDRV_SEQ_IOCTL_CREATE_PORT, seq_ioctl_create_port }, + { SNDRV_SEQ_IOCTL_DELETE_PORT, seq_ioctl_delete_port }, + { SNDRV_SEQ_IOCTL_GET_PORT_INFO, seq_ioctl_get_port_info }, + { SNDRV_SEQ_IOCTL_SET_PORT_INFO, seq_ioctl_set_port_info }, + { SNDRV_SEQ_IOCTL_SUBSCRIBE_PORT, seq_ioctl_subscribe_port }, + { SNDRV_SEQ_IOCTL_UNSUBSCRIBE_PORT, seq_ioctl_unsubscribe_port }, + { SNDRV_SEQ_IOCTL_CREATE_QUEUE, seq_ioctl_create_queue }, + { SNDRV_SEQ_IOCTL_DELETE_QUEUE, seq_ioctl_delete_queue }, + { SNDRV_SEQ_IOCTL_GET_QUEUE_INFO, seq_ioctl_get_queue_info }, + { SNDRV_SEQ_IOCTL_SET_QUEUE_INFO, seq_ioctl_set_queue_info }, + { SNDRV_SEQ_IOCTL_GET_NAMED_QUEUE, seq_ioctl_get_named_queue }, + { SNDRV_SEQ_IOCTL_GET_QUEUE_STATUS, seq_ioctl_get_queue_status }, + { SNDRV_SEQ_IOCTL_GET_QUEUE_TEMPO, seq_ioctl_get_queue_tempo }, + { SNDRV_SEQ_IOCTL_SET_QUEUE_TEMPO, seq_ioctl_set_queue_tempo }, + { SNDRV_SEQ_IOCTL_GET_QUEUE_TIMER, seq_ioctl_get_queue_timer }, + { SNDRV_SEQ_IOCTL_SET_QUEUE_TIMER, seq_ioctl_set_queue_timer }, + { SNDRV_SEQ_IOCTL_GET_QUEUE_CLIENT, seq_ioctl_get_queue_client }, + { SNDRV_SEQ_IOCTL_SET_QUEUE_CLIENT, seq_ioctl_set_queue_client }, + { SNDRV_SEQ_IOCTL_GET_CLIENT_POOL, seq_ioctl_get_client_pool }, + { SNDRV_SEQ_IOCTL_SET_CLIENT_POOL, seq_ioctl_set_client_pool }, + { SNDRV_SEQ_IOCTL_GET_SUBSCRIPTION, seq_ioctl_get_subscription }, + { SNDRV_SEQ_IOCTL_QUERY_NEXT_CLIENT, seq_ioctl_query_next_client }, + { SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT, seq_ioctl_query_next_port }, + { SNDRV_SEQ_IOCTL_REMOVE_EVENTS, seq_ioctl_remove_events }, + { SNDRV_SEQ_IOCTL_QUERY_SUBS, seq_ioctl_query_subs }, { 0, NULL }, };
-static int snd_seq_do_ioctl(struct snd_seq_client *client, unsigned int cmd, +static int seq_do_ioctl(struct snd_seq_client *client, unsigned int cmd, void __user *arg) { const struct seq_ioctl_table *p; @@ -2237,7 +2239,7 @@ static long snd_seq_ioctl(struct file *file, unsigned int cmd, unsigned long arg if (snd_BUG_ON(!client)) return -ENXIO; - return snd_seq_do_ioctl(client, cmd, (void __user *) arg); + return seq_do_ioctl(client, cmd, (void __user *) arg); }
#ifdef CONFIG_COMPAT @@ -2437,7 +2439,7 @@ int snd_seq_kernel_client_ctl(int clientid, unsigned int cmd, void *arg) if (client == NULL) return -ENXIO; fs = snd_enter_user(); - result = snd_seq_do_ioctl(client, cmd, (void __force __user *)arg); + result = seq_do_ioctl(client, cmd, (void __force __user *)arg); snd_leave_user(fs); return result; } diff --git a/sound/core/seq/seq_compat.c b/sound/core/seq/seq_compat.c index 6517590..70d3ddb 100644 --- a/sound/core/seq/seq_compat.c +++ b/sound/core/seq/seq_compat.c @@ -42,8 +42,9 @@ struct snd_seq_port_info32 { char reserved[59]; /* for future use */ };
-static int snd_seq_call_port_info_ioctl(struct snd_seq_client *client, unsigned int cmd, - struct snd_seq_port_info32 __user *data32) +static int seq_call_port_info_ioctl(struct snd_seq_client *client, + unsigned int cmd, + struct snd_seq_port_info32 __user *data32) { int err = -EFAULT; struct snd_seq_port_info *data; @@ -60,7 +61,7 @@ static int snd_seq_call_port_info_ioctl(struct snd_seq_client *client, unsigned data->kernel = NULL;
fs = snd_enter_user(); - err = snd_seq_do_ioctl(client, cmd, data); + err = seq_do_ioctl(client, cmd, data); snd_leave_user(fs); if (err < 0) goto error; @@ -123,17 +124,17 @@ static long snd_seq_ioctl_compat(struct file *file, unsigned int cmd, unsigned l case SNDRV_SEQ_IOCTL_GET_SUBSCRIPTION: case SNDRV_SEQ_IOCTL_QUERY_NEXT_CLIENT: case SNDRV_SEQ_IOCTL_RUNNING_MODE: - return snd_seq_do_ioctl(client, cmd, argp); + return seq_do_ioctl(client, cmd, argp); case SNDRV_SEQ_IOCTL_CREATE_PORT32: - return snd_seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_CREATE_PORT, argp); + return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_CREATE_PORT, argp); case SNDRV_SEQ_IOCTL_DELETE_PORT32: - return snd_seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_DELETE_PORT, argp); + return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_DELETE_PORT, argp); case SNDRV_SEQ_IOCTL_GET_PORT_INFO32: - return snd_seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_GET_PORT_INFO, argp); + return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_GET_PORT_INFO, argp); case SNDRV_SEQ_IOCTL_SET_PORT_INFO32: - return snd_seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_SET_PORT_INFO, argp); + return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_SET_PORT_INFO, argp); case SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT32: - return snd_seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT, argp); + return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT, argp); } return -ENOIOCTLCMD; }
On Sun, 07 Aug 2016 11:48:38 +0200, Takashi Sakamoto wrote:
Exported symbols should have 'snd_' prefix to avoid name conflict, while file local symbols have no need to have. Shorter names are generally preferable due to '80 characters in a line' rule.
I don't think such renames give much benefit unless it dramatically improves the readability.
thanks,
Takashi
This commit renames some local functions.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp
sound/core/seq/seq_clientmgr.c | 180 +++++++++++++++++++++-------------------- sound/core/seq/seq_compat.c | 19 ++--- 2 files changed, 101 insertions(+), 98 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 42be1e5..1331103 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1130,7 +1130,8 @@ static unsigned int snd_seq_poll(struct file *file, poll_table * wait)
/* SYSTEM_INFO ioctl() */ -static int snd_seq_ioctl_system_info(struct snd_seq_client *client, void __user *arg) +static int seq_ioctl_system_info(struct snd_seq_client *client,
void __user *arg){ struct snd_seq_system_info info;
@@ -1150,7 +1151,8 @@ static int snd_seq_ioctl_system_info(struct snd_seq_client *client, void __user
/* RUNNING_MODE ioctl() */ -static int snd_seq_ioctl_running_mode(struct snd_seq_client *client, void __user *arg) +static int seq_ioctl_running_mode(struct snd_seq_client *client,
void __user *arg){ struct snd_seq_running_info info; struct snd_seq_client *cptr; @@ -1213,8 +1215,8 @@ static void get_client_info(struct snd_seq_client *cptr, memset(info->reserved, 0, sizeof(info->reserved)); }
-static int snd_seq_ioctl_get_client_info(struct snd_seq_client *client,
void __user *arg)+static int seq_ioctl_get_client_info(struct snd_seq_client *client,
void __user *arg){ struct snd_seq_client *cptr; struct snd_seq_client_info client_info; @@ -1237,8 +1239,8 @@ static int snd_seq_ioctl_get_client_info(struct snd_seq_client *client,
/* CLIENT_INFO ioctl() */ -static int snd_seq_ioctl_set_client_info(struct snd_seq_client *client,
void __user *arg)+static int seq_ioctl_set_client_info(struct snd_seq_client *client,
void __user *arg){ struct snd_seq_client_info client_info;
@@ -1267,8 +1269,8 @@ static int snd_seq_ioctl_set_client_info(struct snd_seq_client *client, /*
- CREATE PORT ioctl()
*/ -static int snd_seq_ioctl_create_port(struct snd_seq_client *client,
void __user *arg)+static int seq_ioctl_create_port(struct snd_seq_client *client,
void __user *arg){ struct snd_seq_client_port *port; struct snd_seq_port_info info; @@ -1317,8 +1319,8 @@ static int snd_seq_ioctl_create_port(struct snd_seq_client *client, /*
- DELETE PORT ioctl()
*/ -static int snd_seq_ioctl_delete_port(struct snd_seq_client *client,
void __user *arg)+static int seq_ioctl_delete_port(struct snd_seq_client *client,
void __user *arg){ struct snd_seq_port_info info; int err; @@ -1341,8 +1343,8 @@ static int snd_seq_ioctl_delete_port(struct snd_seq_client *client, /*
- GET_PORT_INFO ioctl() (on any client)
*/ -static int snd_seq_ioctl_get_port_info(struct snd_seq_client *client,
void __user *arg)+static int seq_ioctl_get_port_info(struct snd_seq_client *client,
void __user *arg){ struct snd_seq_client *cptr; struct snd_seq_client_port *port; @@ -1374,8 +1376,8 @@ static int snd_seq_ioctl_get_port_info(struct snd_seq_client *client, /*
- SET_PORT_INFO ioctl() (only ports on this/own client)
*/ -static int snd_seq_ioctl_set_port_info(struct snd_seq_client *client,
void __user *arg)+static int seq_ioctl_set_port_info(struct snd_seq_client *client,
void __user *arg){ struct snd_seq_client_port *port; struct snd_seq_port_info info; @@ -1452,8 +1454,8 @@ int snd_seq_client_notify_subscription(int client, int port, /*
- add to port's subscription list IOCTL interface
*/ -static int snd_seq_ioctl_subscribe_port(struct snd_seq_client *client,
void __user *arg)+static int seq_ioctl_subscribe_port(struct snd_seq_client *client,
void __user *arg){ int result = -EINVAL; struct snd_seq_client *receiver = NULL, *sender = NULL; @@ -1497,8 +1499,8 @@ static int snd_seq_ioctl_subscribe_port(struct snd_seq_client *client, /*
- remove from port's subscription list
*/ -static int snd_seq_ioctl_unsubscribe_port(struct snd_seq_client *client,
void __user *arg)+static int seq_ioctl_unsubscribe_port(struct snd_seq_client *client,
void __user *arg){ int result = -ENXIO; struct snd_seq_client *receiver = NULL, *sender = NULL; @@ -1539,8 +1541,8 @@ static int snd_seq_ioctl_unsubscribe_port(struct snd_seq_client *client,
/* CREATE_QUEUE ioctl() */ -static int snd_seq_ioctl_create_queue(struct snd_seq_client *client,
void __user *arg)+static int seq_ioctl_create_queue(struct snd_seq_client *client,
void __user *arg){ struct snd_seq_queue_info info; int result; @@ -1574,8 +1576,8 @@ static int snd_seq_ioctl_create_queue(struct snd_seq_client *client, }
/* DELETE_QUEUE ioctl() */ -static int snd_seq_ioctl_delete_queue(struct snd_seq_client *client,
void __user *arg)+static int seq_ioctl_delete_queue(struct snd_seq_client *client,
void __user *arg){ struct snd_seq_queue_info info;
@@ -1586,8 +1588,8 @@ static int snd_seq_ioctl_delete_queue(struct snd_seq_client *client, }
/* GET_QUEUE_INFO ioctl() */ -static int snd_seq_ioctl_get_queue_info(struct snd_seq_client *client,
void __user *arg)+static int seq_ioctl_get_queue_info(struct snd_seq_client *client,
void __user *arg){ struct snd_seq_queue_info info; struct snd_seq_queue *q; @@ -1613,8 +1615,8 @@ static int snd_seq_ioctl_get_queue_info(struct snd_seq_client *client, }
/* SET_QUEUE_INFO ioctl() */ -static int snd_seq_ioctl_set_queue_info(struct snd_seq_client *client,
void __user *arg)+static int seq_ioctl_set_queue_info(struct snd_seq_client *client,
void __user *arg){ struct snd_seq_queue_info info; struct snd_seq_queue *q; @@ -1649,7 +1651,8 @@ static int snd_seq_ioctl_set_queue_info(struct snd_seq_client *client, }
/* GET_NAMED_QUEUE ioctl() */ -static int snd_seq_ioctl_get_named_queue(struct snd_seq_client *client, void __user *arg) +static int seq_ioctl_get_named_queue(struct snd_seq_client *client,
void __user *arg){ struct snd_seq_queue_info info; struct snd_seq_queue *q; @@ -1672,8 +1675,8 @@ static int snd_seq_ioctl_get_named_queue(struct snd_seq_client *client, void __u }
/* GET_QUEUE_STATUS ioctl() */ -static int snd_seq_ioctl_get_queue_status(struct snd_seq_client *client,
void __user *arg)+static int seq_ioctl_get_queue_status(struct snd_seq_client *client,
void __user *arg){ struct snd_seq_queue_status status; struct snd_seq_queue *queue; @@ -1706,8 +1709,8 @@ static int snd_seq_ioctl_get_queue_status(struct snd_seq_client *client,
/* GET_QUEUE_TEMPO ioctl() */ -static int snd_seq_ioctl_get_queue_tempo(struct snd_seq_client *client,
void __user *arg)+static int seq_ioctl_get_queue_tempo(struct snd_seq_client *client,
void __user *arg){ struct snd_seq_queue_tempo tempo; struct snd_seq_queue *queue; @@ -1746,8 +1749,8 @@ int snd_seq_set_queue_tempo(int client, struct snd_seq_queue_tempo *tempo)
EXPORT_SYMBOL(snd_seq_set_queue_tempo);
-static int snd_seq_ioctl_set_queue_tempo(struct snd_seq_client *client,
void __user *arg)+static int seq_ioctl_set_queue_tempo(struct snd_seq_client *client,
void __user *arg){ int result; struct snd_seq_queue_tempo tempo; @@ -1761,8 +1764,8 @@ static int snd_seq_ioctl_set_queue_tempo(struct snd_seq_client *client,
/* GET_QUEUE_TIMER ioctl() */ -static int snd_seq_ioctl_get_queue_timer(struct snd_seq_client *client,
void __user *arg)+static int seq_ioctl_get_queue_timer(struct snd_seq_client *client,
void __user *arg){ struct snd_seq_queue_timer timer; struct snd_seq_queue *queue; @@ -1798,8 +1801,8 @@ static int snd_seq_ioctl_get_queue_timer(struct snd_seq_client *client,
/* SET_QUEUE_TIMER ioctl() */ -static int snd_seq_ioctl_set_queue_timer(struct snd_seq_client *client,
void __user *arg)+static int seq_ioctl_set_queue_timer(struct snd_seq_client *client,
void __user *arg){ int result = 0; struct snd_seq_queue_timer timer; @@ -1840,8 +1843,8 @@ static int snd_seq_ioctl_set_queue_timer(struct snd_seq_client *client,
/* GET_QUEUE_CLIENT ioctl() */ -static int snd_seq_ioctl_get_queue_client(struct snd_seq_client *client,
void __user *arg)+static int seq_ioctl_get_queue_client(struct snd_seq_client *client,
void __user *arg){ struct snd_seq_queue_client info; int used; @@ -1862,8 +1865,8 @@ static int snd_seq_ioctl_get_queue_client(struct snd_seq_client *client,
/* SET_QUEUE_CLIENT ioctl() */ -static int snd_seq_ioctl_set_queue_client(struct snd_seq_client *client,
void __user *arg)+static int seq_ioctl_set_queue_client(struct snd_seq_client *client,
void __user *arg){ int err; struct snd_seq_queue_client info; @@ -1877,13 +1880,13 @@ static int snd_seq_ioctl_set_queue_client(struct snd_seq_client *client, return err; }
- return snd_seq_ioctl_get_queue_client(client, arg);
- return seq_ioctl_get_queue_client(client, arg);
}
/* GET_CLIENT_POOL ioctl() */ -static int snd_seq_ioctl_get_client_pool(struct snd_seq_client *client,
void __user *arg)+static int seq_ioctl_get_client_pool(struct snd_seq_client *client,
void __user *arg){ struct snd_seq_client_pool info; struct snd_seq_client *cptr; @@ -1917,8 +1920,8 @@ static int snd_seq_ioctl_get_client_pool(struct snd_seq_client *client, }
/* SET_CLIENT_POOL ioctl() */ -static int snd_seq_ioctl_set_client_pool(struct snd_seq_client *client,
void __user *arg)+static int seq_ioctl_set_client_pool(struct snd_seq_client *client,
void __user *arg){ struct snd_seq_client_pool info; int rc; @@ -1957,13 +1960,13 @@ static int snd_seq_ioctl_set_client_pool(struct snd_seq_client *client, client->pool->room = info.output_room; }
- return snd_seq_ioctl_get_client_pool(client, arg);
- return seq_ioctl_get_client_pool(client, arg);
}
/* REMOVE_EVENTS ioctl() */ -static int snd_seq_ioctl_remove_events(struct snd_seq_client *client,
void __user *arg)+static int seq_ioctl_remove_events(struct snd_seq_client *client,
void __user *arg){ struct snd_seq_remove_events info;
@@ -1992,8 +1995,8 @@ static int snd_seq_ioctl_remove_events(struct snd_seq_client *client, /*
- get subscription info
*/ -static int snd_seq_ioctl_get_subscription(struct snd_seq_client *client,
void __user *arg)+static int seq_ioctl_get_subscription(struct snd_seq_client *client,
void __user *arg){ int result; struct snd_seq_client *sender = NULL; @@ -2032,8 +2035,7 @@ static int snd_seq_ioctl_get_subscription(struct snd_seq_client *client, /*
- get subscription info - check only its presence
*/ -static int snd_seq_ioctl_query_subs(struct snd_seq_client *client,
void __user *arg)+static int seq_ioctl_query_subs(struct snd_seq_client *client, void __user *arg) { int result = -ENXIO; struct snd_seq_client *cptr = NULL; @@ -2102,8 +2104,8 @@ static int snd_seq_ioctl_query_subs(struct snd_seq_client *client, /*
- query next client
*/ -static int snd_seq_ioctl_query_next_client(struct snd_seq_client *client,
void __user *arg)+static int seq_ioctl_query_next_client(struct snd_seq_client *client,
void __user *arg){ struct snd_seq_client *cptr = NULL; struct snd_seq_client_info info; @@ -2134,8 +2136,8 @@ static int snd_seq_ioctl_query_next_client(struct snd_seq_client *client, /*
- query next port
*/ -static int snd_seq_ioctl_query_next_port(struct snd_seq_client *client,
void __user *arg)+static int seq_ioctl_query_next_port(struct snd_seq_client *client,
void __user *arg){ struct snd_seq_client *cptr; struct snd_seq_client_port *port = NULL; @@ -2172,39 +2174,39 @@ static const struct seq_ioctl_table { unsigned int cmd; int (*func)(struct snd_seq_client *client, void __user * arg); } ioctl_tables[] = {
- { SNDRV_SEQ_IOCTL_SYSTEM_INFO, snd_seq_ioctl_system_info },
- { SNDRV_SEQ_IOCTL_RUNNING_MODE, snd_seq_ioctl_running_mode },
- { SNDRV_SEQ_IOCTL_GET_CLIENT_INFO, snd_seq_ioctl_get_client_info },
- { SNDRV_SEQ_IOCTL_SET_CLIENT_INFO, snd_seq_ioctl_set_client_info },
- { SNDRV_SEQ_IOCTL_CREATE_PORT, snd_seq_ioctl_create_port },
- { SNDRV_SEQ_IOCTL_DELETE_PORT, snd_seq_ioctl_delete_port },
- { SNDRV_SEQ_IOCTL_GET_PORT_INFO, snd_seq_ioctl_get_port_info },
- { SNDRV_SEQ_IOCTL_SET_PORT_INFO, snd_seq_ioctl_set_port_info },
- { SNDRV_SEQ_IOCTL_SUBSCRIBE_PORT, snd_seq_ioctl_subscribe_port },
- { SNDRV_SEQ_IOCTL_UNSUBSCRIBE_PORT, snd_seq_ioctl_unsubscribe_port },
- { SNDRV_SEQ_IOCTL_CREATE_QUEUE, snd_seq_ioctl_create_queue },
- { SNDRV_SEQ_IOCTL_DELETE_QUEUE, snd_seq_ioctl_delete_queue },
- { SNDRV_SEQ_IOCTL_GET_QUEUE_INFO, snd_seq_ioctl_get_queue_info },
- { SNDRV_SEQ_IOCTL_SET_QUEUE_INFO, snd_seq_ioctl_set_queue_info },
- { SNDRV_SEQ_IOCTL_GET_NAMED_QUEUE, snd_seq_ioctl_get_named_queue },
- { SNDRV_SEQ_IOCTL_GET_QUEUE_STATUS, snd_seq_ioctl_get_queue_status },
- { SNDRV_SEQ_IOCTL_GET_QUEUE_TEMPO, snd_seq_ioctl_get_queue_tempo },
- { SNDRV_SEQ_IOCTL_SET_QUEUE_TEMPO, snd_seq_ioctl_set_queue_tempo },
- { SNDRV_SEQ_IOCTL_GET_QUEUE_TIMER, snd_seq_ioctl_get_queue_timer },
- { SNDRV_SEQ_IOCTL_SET_QUEUE_TIMER, snd_seq_ioctl_set_queue_timer },
- { SNDRV_SEQ_IOCTL_GET_QUEUE_CLIENT, snd_seq_ioctl_get_queue_client },
- { SNDRV_SEQ_IOCTL_SET_QUEUE_CLIENT, snd_seq_ioctl_set_queue_client },
- { SNDRV_SEQ_IOCTL_GET_CLIENT_POOL, snd_seq_ioctl_get_client_pool },
- { SNDRV_SEQ_IOCTL_SET_CLIENT_POOL, snd_seq_ioctl_set_client_pool },
- { SNDRV_SEQ_IOCTL_GET_SUBSCRIPTION, snd_seq_ioctl_get_subscription },
- { SNDRV_SEQ_IOCTL_QUERY_NEXT_CLIENT, snd_seq_ioctl_query_next_client },
- { SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT, snd_seq_ioctl_query_next_port },
- { SNDRV_SEQ_IOCTL_REMOVE_EVENTS, snd_seq_ioctl_remove_events },
- { SNDRV_SEQ_IOCTL_QUERY_SUBS, snd_seq_ioctl_query_subs },
- { SNDRV_SEQ_IOCTL_SYSTEM_INFO, seq_ioctl_system_info },
- { SNDRV_SEQ_IOCTL_RUNNING_MODE, seq_ioctl_running_mode },
- { SNDRV_SEQ_IOCTL_GET_CLIENT_INFO, seq_ioctl_get_client_info },
- { SNDRV_SEQ_IOCTL_SET_CLIENT_INFO, seq_ioctl_set_client_info },
- { SNDRV_SEQ_IOCTL_CREATE_PORT, seq_ioctl_create_port },
- { SNDRV_SEQ_IOCTL_DELETE_PORT, seq_ioctl_delete_port },
- { SNDRV_SEQ_IOCTL_GET_PORT_INFO, seq_ioctl_get_port_info },
- { SNDRV_SEQ_IOCTL_SET_PORT_INFO, seq_ioctl_set_port_info },
- { SNDRV_SEQ_IOCTL_SUBSCRIBE_PORT, seq_ioctl_subscribe_port },
- { SNDRV_SEQ_IOCTL_UNSUBSCRIBE_PORT, seq_ioctl_unsubscribe_port },
- { SNDRV_SEQ_IOCTL_CREATE_QUEUE, seq_ioctl_create_queue },
- { SNDRV_SEQ_IOCTL_DELETE_QUEUE, seq_ioctl_delete_queue },
- { SNDRV_SEQ_IOCTL_GET_QUEUE_INFO, seq_ioctl_get_queue_info },
- { SNDRV_SEQ_IOCTL_SET_QUEUE_INFO, seq_ioctl_set_queue_info },
- { SNDRV_SEQ_IOCTL_GET_NAMED_QUEUE, seq_ioctl_get_named_queue },
- { SNDRV_SEQ_IOCTL_GET_QUEUE_STATUS, seq_ioctl_get_queue_status },
- { SNDRV_SEQ_IOCTL_GET_QUEUE_TEMPO, seq_ioctl_get_queue_tempo },
- { SNDRV_SEQ_IOCTL_SET_QUEUE_TEMPO, seq_ioctl_set_queue_tempo },
- { SNDRV_SEQ_IOCTL_GET_QUEUE_TIMER, seq_ioctl_get_queue_timer },
- { SNDRV_SEQ_IOCTL_SET_QUEUE_TIMER, seq_ioctl_set_queue_timer },
- { SNDRV_SEQ_IOCTL_GET_QUEUE_CLIENT, seq_ioctl_get_queue_client },
- { SNDRV_SEQ_IOCTL_SET_QUEUE_CLIENT, seq_ioctl_set_queue_client },
- { SNDRV_SEQ_IOCTL_GET_CLIENT_POOL, seq_ioctl_get_client_pool },
- { SNDRV_SEQ_IOCTL_SET_CLIENT_POOL, seq_ioctl_set_client_pool },
- { SNDRV_SEQ_IOCTL_GET_SUBSCRIPTION, seq_ioctl_get_subscription },
- { SNDRV_SEQ_IOCTL_QUERY_NEXT_CLIENT, seq_ioctl_query_next_client },
- { SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT, seq_ioctl_query_next_port },
- { SNDRV_SEQ_IOCTL_REMOVE_EVENTS, seq_ioctl_remove_events },
- { SNDRV_SEQ_IOCTL_QUERY_SUBS, seq_ioctl_query_subs }, { 0, NULL },
};
-static int snd_seq_do_ioctl(struct snd_seq_client *client, unsigned int cmd, +static int seq_do_ioctl(struct snd_seq_client *client, unsigned int cmd, void __user *arg) { const struct seq_ioctl_table *p; @@ -2237,7 +2239,7 @@ static long snd_seq_ioctl(struct file *file, unsigned int cmd, unsigned long arg if (snd_BUG_ON(!client)) return -ENXIO;
- return snd_seq_do_ioctl(client, cmd, (void __user *) arg);
- return seq_do_ioctl(client, cmd, (void __user *) arg);
}
#ifdef CONFIG_COMPAT @@ -2437,7 +2439,7 @@ int snd_seq_kernel_client_ctl(int clientid, unsigned int cmd, void *arg) if (client == NULL) return -ENXIO; fs = snd_enter_user();
- result = snd_seq_do_ioctl(client, cmd, (void __force __user *)arg);
- result = seq_do_ioctl(client, cmd, (void __force __user *)arg); snd_leave_user(fs); return result;
} diff --git a/sound/core/seq/seq_compat.c b/sound/core/seq/seq_compat.c index 6517590..70d3ddb 100644 --- a/sound/core/seq/seq_compat.c +++ b/sound/core/seq/seq_compat.c @@ -42,8 +42,9 @@ struct snd_seq_port_info32 { char reserved[59]; /* for future use */ };
-static int snd_seq_call_port_info_ioctl(struct snd_seq_client *client, unsigned int cmd,
struct snd_seq_port_info32 __user *data32)+static int seq_call_port_info_ioctl(struct snd_seq_client *client,
unsigned int cmd,struct snd_seq_port_info32 __user *data32){ int err = -EFAULT; struct snd_seq_port_info *data; @@ -60,7 +61,7 @@ static int snd_seq_call_port_info_ioctl(struct snd_seq_client *client, unsigned data->kernel = NULL;
fs = snd_enter_user();
- err = snd_seq_do_ioctl(client, cmd, data);
- err = seq_do_ioctl(client, cmd, data); snd_leave_user(fs); if (err < 0) goto error;
@@ -123,17 +124,17 @@ static long snd_seq_ioctl_compat(struct file *file, unsigned int cmd, unsigned l case SNDRV_SEQ_IOCTL_GET_SUBSCRIPTION: case SNDRV_SEQ_IOCTL_QUERY_NEXT_CLIENT: case SNDRV_SEQ_IOCTL_RUNNING_MODE:
return snd_seq_do_ioctl(client, cmd, argp);
return seq_do_ioctl(client, cmd, argp);
return snd_seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_CREATE_PORT, argp);
return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_CREATE_PORT, argp);
return snd_seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_DELETE_PORT, argp);
return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_DELETE_PORT, argp);
return snd_seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_GET_PORT_INFO, argp);
return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_GET_PORT_INFO, argp);
return snd_seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_SET_PORT_INFO, argp);
return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_SET_PORT_INFO, argp);
return snd_seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT, argp);
return seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT, argp);}
2.7.4
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Most of callback functions corresponding to each ioctl command are in local table, while two of them are not. This is a bit worse to handle the command in a consistent way.
This commit adds entries for these two functions in the table.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 1331103..d6c1219 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1129,6 +1129,16 @@ static unsigned int snd_seq_poll(struct file *file, poll_table * wait) /*-----------------------------------------------------*/
+static int seq_ioctl_pversion(struct snd_seq_client *client, void __user *arg) +{ + return put_user(SNDRV_SEQ_VERSION, (int __user *)arg) ? -EFAULT : 0; +} + +static int seq_ioctl_client_id(struct snd_seq_client *client, void __user *arg) +{ + return put_user(client->number, (int __user *)arg) ? -EFAULT : 0; +} + /* SYSTEM_INFO ioctl() */ static int seq_ioctl_system_info(struct snd_seq_client *client, void __user *arg) @@ -2174,6 +2184,8 @@ static const struct seq_ioctl_table { unsigned int cmd; int (*func)(struct snd_seq_client *client, void __user * arg); } ioctl_tables[] = { + { SNDRV_SEQ_IOCTL_PVERSION, seq_ioctl_pversion }, + { SNDRV_SEQ_IOCTL_CLIENT_ID, seq_ioctl_client_id }, { SNDRV_SEQ_IOCTL_SYSTEM_INFO, seq_ioctl_system_info }, { SNDRV_SEQ_IOCTL_RUNNING_MODE, seq_ioctl_running_mode }, { SNDRV_SEQ_IOCTL_GET_CLIENT_INFO, seq_ioctl_get_client_info }, @@ -2211,15 +2223,6 @@ static int seq_do_ioctl(struct snd_seq_client *client, unsigned int cmd, { const struct seq_ioctl_table *p;
- switch (cmd) { - case SNDRV_SEQ_IOCTL_PVERSION: - /* return sequencer version number */ - return put_user(SNDRV_SEQ_VERSION, (int __user *)arg) ? -EFAULT : 0; - case SNDRV_SEQ_IOCTL_CLIENT_ID: - /* return the id of this client */ - return put_user(client->number, (int __user *)arg) ? -EFAULT : 0; - } - if (! arg) return -EFAULT; for (p = ioctl_tables; p->cmd; p++) {
ALSA sequencer core allows kernel-mode tasks to execute operations corresponding to ioctls by user-mode tasks. This is for compatibility layers such as ABIs for 32/64 bit and I/O operations via Open Sound System.
This design requires to handle ioctl data in kernel space. For this requirement, ALSA sequencer core temporarily changes address limit for the task by get_fs()/set_fs() macro.
Although, this way get shape of code worse, because even when an pointer has '__user' qualifier, actually it refers to kernel space, depending on the task. This easily misleads readers.
This commit is a preparation for following patches to solve this issue. Data from user space is once copied to kernel stack, then operated and copied to user space, in a consistent manner. This manner forces all ioctl operations to copy the data from/to user space, even if it's read-only or write-only. Thus, it has an overhead for simpler ioctl commands.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 136 ++++++++++++++++++++++++++++++----------- 1 file changed, 101 insertions(+), 35 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index d6c1219..17d988a 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -2182,40 +2182,70 @@ static int seq_ioctl_query_next_port(struct snd_seq_client *client,
static const struct seq_ioctl_table { unsigned int cmd; - int (*func)(struct snd_seq_client *client, void __user * arg); + int (*func)(struct snd_seq_client *client, void *arg); + size_t arg_size; } ioctl_tables[] = { - { SNDRV_SEQ_IOCTL_PVERSION, seq_ioctl_pversion }, - { SNDRV_SEQ_IOCTL_CLIENT_ID, seq_ioctl_client_id }, - { SNDRV_SEQ_IOCTL_SYSTEM_INFO, seq_ioctl_system_info }, - { SNDRV_SEQ_IOCTL_RUNNING_MODE, seq_ioctl_running_mode }, - { SNDRV_SEQ_IOCTL_GET_CLIENT_INFO, seq_ioctl_get_client_info }, - { SNDRV_SEQ_IOCTL_SET_CLIENT_INFO, seq_ioctl_set_client_info }, - { SNDRV_SEQ_IOCTL_CREATE_PORT, seq_ioctl_create_port }, - { SNDRV_SEQ_IOCTL_DELETE_PORT, seq_ioctl_delete_port }, - { SNDRV_SEQ_IOCTL_GET_PORT_INFO, seq_ioctl_get_port_info }, - { SNDRV_SEQ_IOCTL_SET_PORT_INFO, seq_ioctl_set_port_info }, - { SNDRV_SEQ_IOCTL_SUBSCRIBE_PORT, seq_ioctl_subscribe_port }, - { SNDRV_SEQ_IOCTL_UNSUBSCRIBE_PORT, seq_ioctl_unsubscribe_port }, - { SNDRV_SEQ_IOCTL_CREATE_QUEUE, seq_ioctl_create_queue }, - { SNDRV_SEQ_IOCTL_DELETE_QUEUE, seq_ioctl_delete_queue }, - { SNDRV_SEQ_IOCTL_GET_QUEUE_INFO, seq_ioctl_get_queue_info }, - { SNDRV_SEQ_IOCTL_SET_QUEUE_INFO, seq_ioctl_set_queue_info }, - { SNDRV_SEQ_IOCTL_GET_NAMED_QUEUE, seq_ioctl_get_named_queue }, - { SNDRV_SEQ_IOCTL_GET_QUEUE_STATUS, seq_ioctl_get_queue_status }, - { SNDRV_SEQ_IOCTL_GET_QUEUE_TEMPO, seq_ioctl_get_queue_tempo }, - { SNDRV_SEQ_IOCTL_SET_QUEUE_TEMPO, seq_ioctl_set_queue_tempo }, - { SNDRV_SEQ_IOCTL_GET_QUEUE_TIMER, seq_ioctl_get_queue_timer }, - { SNDRV_SEQ_IOCTL_SET_QUEUE_TIMER, seq_ioctl_set_queue_timer }, - { SNDRV_SEQ_IOCTL_GET_QUEUE_CLIENT, seq_ioctl_get_queue_client }, - { SNDRV_SEQ_IOCTL_SET_QUEUE_CLIENT, seq_ioctl_set_queue_client }, - { SNDRV_SEQ_IOCTL_GET_CLIENT_POOL, seq_ioctl_get_client_pool }, - { SNDRV_SEQ_IOCTL_SET_CLIENT_POOL, seq_ioctl_set_client_pool }, - { SNDRV_SEQ_IOCTL_GET_SUBSCRIPTION, seq_ioctl_get_subscription }, - { SNDRV_SEQ_IOCTL_QUERY_NEXT_CLIENT, seq_ioctl_query_next_client }, - { SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT, seq_ioctl_query_next_port }, - { SNDRV_SEQ_IOCTL_REMOVE_EVENTS, seq_ioctl_remove_events }, - { SNDRV_SEQ_IOCTL_QUERY_SUBS, seq_ioctl_query_subs }, - { 0, NULL }, + { SNDRV_SEQ_IOCTL_PVERSION, seq_ioctl_pversion, sizeof(int) }, + { SNDRV_SEQ_IOCTL_CLIENT_ID, seq_ioctl_client_id, sizeof(int) }, + { SNDRV_SEQ_IOCTL_SYSTEM_INFO, seq_ioctl_system_info, + sizeof(struct snd_seq_system_info) }, + { SNDRV_SEQ_IOCTL_RUNNING_MODE, seq_ioctl_running_mode, + sizeof(struct snd_seq_running_info) }, + { SNDRV_SEQ_IOCTL_GET_CLIENT_INFO, seq_ioctl_get_client_info, + sizeof(struct snd_seq_client_info) }, + { SNDRV_SEQ_IOCTL_SET_CLIENT_INFO, seq_ioctl_set_client_info, + sizeof(struct snd_seq_client_info) }, + { SNDRV_SEQ_IOCTL_CREATE_PORT, seq_ioctl_create_port, + sizeof(struct snd_seq_port_info) }, + { SNDRV_SEQ_IOCTL_DELETE_PORT, seq_ioctl_delete_port, + sizeof(struct snd_seq_port_info) }, + { SNDRV_SEQ_IOCTL_GET_PORT_INFO, seq_ioctl_get_port_info, + sizeof(struct snd_seq_port_info) }, + { SNDRV_SEQ_IOCTL_SET_PORT_INFO, seq_ioctl_set_port_info, + sizeof(struct snd_seq_port_info) }, + { SNDRV_SEQ_IOCTL_SUBSCRIBE_PORT, seq_ioctl_subscribe_port, + sizeof(struct snd_seq_port_subscribe) }, + { SNDRV_SEQ_IOCTL_UNSUBSCRIBE_PORT, seq_ioctl_unsubscribe_port, + sizeof(struct snd_seq_port_subscribe) }, + { SNDRV_SEQ_IOCTL_CREATE_QUEUE, seq_ioctl_create_queue, + sizeof(struct snd_seq_queue_info) }, + { SNDRV_SEQ_IOCTL_DELETE_QUEUE, seq_ioctl_delete_queue, + sizeof(struct snd_seq_queue_info) }, + { SNDRV_SEQ_IOCTL_GET_QUEUE_INFO, seq_ioctl_get_queue_info, + sizeof(struct snd_seq_queue_info) }, + { SNDRV_SEQ_IOCTL_SET_QUEUE_INFO, seq_ioctl_set_queue_info, + sizeof(struct snd_seq_queue_info) }, + { SNDRV_SEQ_IOCTL_GET_NAMED_QUEUE, seq_ioctl_get_named_queue, + sizeof(struct snd_seq_queue_info) }, + { SNDRV_SEQ_IOCTL_GET_QUEUE_STATUS, seq_ioctl_get_queue_status, + sizeof(struct snd_seq_queue_status) }, + { SNDRV_SEQ_IOCTL_GET_QUEUE_TEMPO, seq_ioctl_get_queue_tempo, + sizeof(struct snd_seq_queue_tempo) }, + { SNDRV_SEQ_IOCTL_SET_QUEUE_TEMPO, seq_ioctl_set_queue_tempo, + sizeof(struct snd_seq_queue_tempo) }, + { SNDRV_SEQ_IOCTL_GET_QUEUE_TIMER, seq_ioctl_get_queue_timer, + sizeof(struct snd_seq_queue_timer) }, + { SNDRV_SEQ_IOCTL_SET_QUEUE_TIMER, seq_ioctl_set_queue_timer, + sizeof(struct snd_seq_queue_timer) }, + { SNDRV_SEQ_IOCTL_GET_QUEUE_CLIENT, seq_ioctl_get_queue_client, + sizeof(struct snd_seq_queue_client) }, + { SNDRV_SEQ_IOCTL_SET_QUEUE_CLIENT, seq_ioctl_set_queue_client, + sizeof(struct snd_seq_queue_client) }, + { SNDRV_SEQ_IOCTL_GET_CLIENT_POOL, seq_ioctl_get_client_pool, + sizeof(struct snd_seq_client_pool) }, + { SNDRV_SEQ_IOCTL_SET_CLIENT_POOL, seq_ioctl_set_client_pool, + sizeof(struct snd_seq_client_pool) }, + { SNDRV_SEQ_IOCTL_GET_SUBSCRIPTION, seq_ioctl_get_subscription, + sizeof(struct snd_seq_port_subscribe) }, + { SNDRV_SEQ_IOCTL_QUERY_NEXT_CLIENT, seq_ioctl_query_next_client, + sizeof(struct snd_seq_client_info) }, + { SNDRV_SEQ_IOCTL_QUERY_NEXT_PORT, seq_ioctl_query_next_port, + sizeof(struct snd_seq_port_info) }, + { SNDRV_SEQ_IOCTL_REMOVE_EVENTS, seq_ioctl_remove_events, + sizeof(struct snd_seq_remove_events) }, + { SNDRV_SEQ_IOCTL_QUERY_SUBS, seq_ioctl_query_subs, + sizeof(struct snd_seq_query_subs) }, + { 0, NULL, 0 }, };
static int seq_do_ioctl(struct snd_seq_client *client, unsigned int cmd, @@ -2235,14 +2265,50 @@ static int seq_do_ioctl(struct snd_seq_client *client, unsigned int cmd, }
-static long snd_seq_ioctl(struct file *file, unsigned int cmd, unsigned long arg) +static long snd_seq_ioctl(struct file *file, unsigned int cmd, + unsigned long arg) { struct snd_seq_client *client = file->private_data; + const struct seq_ioctl_table *p; + union ioctl_arg { + int pversion; + int client_id; + struct snd_seq_system_info system_info; + struct snd_seq_running_info running_info; + struct snd_seq_client_info client_info; + struct snd_seq_port_info port_info; + struct snd_seq_port_subscribe port_subscribe; + struct snd_seq_queue_info queue_info; + struct snd_seq_queue_status queue_status; + struct snd_seq_queue_tempo tempo; + struct snd_seq_queue_timer queue_timer; + struct snd_seq_queue_client queue_client; + struct snd_seq_client_pool client_pool; + struct snd_seq_remove_events remove_events; + struct snd_seq_query_subs query_subs; + } buf = {0}; + int err;
if (snd_BUG_ON(!client)) return -ENXIO; + + for (p = ioctl_tables; p->cmd > 0; ++p) { + if (p->cmd == cmd) + break; + } + if (p->cmd == 0) + return -ENOTTY; + + if (copy_from_user(&buf, (const void __user *)arg, p->arg_size)) + return -EFAULT; - return seq_do_ioctl(client, cmd, (void __user *) arg); + err = p->func(client, &buf); + if (err >= 0) { + if (copy_to_user((void __user *)arg, &buf, p->arg_size)) + return -EFAULT; + } + + return err; }
#ifdef CONFIG_COMPAT
Takashi Sakamoto wrote:
Data from user space is once copied to kernel stack, then operated and copied to user space, in a consistent manner. This manner forces all ioctl operations to copy the data from/to user space, even if it's read-only or write-only. Thus, it has an overhead for simpler ioctl commands.
The ioctl code itself already contains information about the direction and size of the data to be copied (and in theory, these values are correct). See dispatch_ioctl() in drivers/firewire/core-cdev.c for an example.
Regards, Clemens
Hi Clemens,
On Aug 7 2016 19:15, Clemens Ladisch wrote:
Takashi Sakamoto wrote:
Data from user space is once copied to kernel stack, then operated and copied to user space, in a consistent manner. This manner forces all ioctl operations to copy the data from/to user space, even if it's read-only or write-only. Thus, it has an overhead for simpler ioctl commands.
The ioctl code itself already contains information about the direction and size of the data to be copied (and in theory, these values are correct). See dispatch_ioctl() in drivers/firewire/core-cdev.c for an example.
A nice idea.
_IOC_SIZE macro pick up 13 or 14 bits (architecture-dependent) in ioctl command, which represents the size of argument. In my patch, the size of 'union ioctl_arg' is 188 (x86/x32) or 192 (x86_64) and there's enough rest of the size field. So we can pick up the size from ioctl command by the macro because the size represents the maximum bytes of argument for all of sequencer ioctls.
I'll post revised version tomorrow. Thanks ;)
Takashi Sakamoto
On Sun, 07 Aug 2016 16:26:35 +0200, Takashi Sakamoto wrote:
Hi Clemens,
On Aug 7 2016 19:15, Clemens Ladisch wrote:
Takashi Sakamoto wrote:
Data from user space is once copied to kernel stack, then operated and copied to user space, in a consistent manner. This manner forces all ioctl operations to copy the data from/to user space, even if it's read-only or write-only. Thus, it has an overhead for simpler ioctl commands.
The ioctl code itself already contains information about the direction and size of the data to be copied (and in theory, these values are correct). See dispatch_ioctl() in drivers/firewire/core-cdev.c for an example.
A nice idea.
_IOC_SIZE macro pick up 13 or 14 bits (architecture-dependent) in ioctl command, which represents the size of argument. In my patch, the size of 'union ioctl_arg' is 188 (x86/x32) or 192 (x86_64) and there's enough rest of the size field. So we can pick up the size from ioctl command by the macro because the size represents the maximum bytes of argument for all of sequencer ioctls.
It's not only about the size. It contains the r/w bits, so you can avoid the unnecessary user-copy calls, too.
Takashi
On Aug 8 2016 16:10, Takashi Iwai wrote:
_IOC_SIZE macro pick up 13 or 14 bits (architecture-dependent) in ioctl command, which represents the size of argument. In my patch, the size of 'union ioctl_arg' is 188 (x86/x32) or 192 (x86_64) and there's enough rest of the size field. So we can pick up the size from ioctl command by the macro because the size represents the maximum bytes of argument for all of sequencer ioctls.
It's not only about the size. It contains the r/w bits, so you can avoid the unnecessary user-copy calls, too.
SET_QUEUE_CLIENT ioctl command is defined as 'W', while a corresponding function writes to userspace. This is unavoidable bug.
Regards
Takashi Sakamoto
On Mon, 08 Aug 2016 15:46:21 +0200, Takashi Sakamoto wrote:
On Aug 8 2016 16:10, Takashi Iwai wrote:
_IOC_SIZE macro pick up 13 or 14 bits (architecture-dependent) in ioctl command, which represents the size of argument. In my patch, the size of 'union ioctl_arg' is 188 (x86/x32) or 192 (x86_64) and there's enough rest of the size field. So we can pick up the size from ioctl command by the macro because the size represents the maximum bytes of argument for all of sequencer ioctls.
It's not only about the size. It contains the r/w bits, so you can avoid the unnecessary user-copy calls, too.
SET_QUEUE_CLIENT ioctl command is defined as 'W', while a corresponding function writes to userspace. This is unavoidable bug.
Yes, there are a few things to be corrected. That is, some ioctls need to be remapped to right ioctl numbers beforehand.
Takashi
This kernel API is used by kernel implementation. Currently, it's used for kernel clients of ALSA sequencer, while it can be used for application clients. The difference is just on address spaces of argument. In short, this kernel API can be available for application client with data in kernel space.
This commit adds a document about this.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 17d988a..a09cb84 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -2494,9 +2494,17 @@ int snd_seq_kernel_client_dispatch(int client, struct snd_seq_event * ev,
EXPORT_SYMBOL(snd_seq_kernel_client_dispatch);
-/* - * exported, called by kernel clients to perform same functions as with - * userland ioctl() +/** + * snd_seq_kernel_client_ctl - operate a command for a client with data in + * kernel space. + * @clientid: A numerical ID for a client. + * @cmd: An ioctl(2) command for ALSA sequencer operation. + * @arg: A pointer to data in kernel space. + * + * Against its name, both kernel/application client can be handled by this + * kernel API. Address space for 'arg' argument should be in kernel space. + * + * Return: 0 at success. Minus error code at failure. */ int snd_seq_kernel_client_ctl(int clientid, unsigned int cmd, void *arg) {
On Sun, 07 Aug 2016 11:48:41 +0200, Takashi Sakamoto wrote:
This kernel API is used by kernel implementation. Currently, it's used for kernel clients of ALSA sequencer, while it can be used for application clients. The difference is just on address spaces of argument. In short, this kernel API can be available for application client with data in kernel space.
This commit adds a document about this.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp
sound/core/seq/seq_clientmgr.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 17d988a..a09cb84 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -2494,9 +2494,17 @@ int snd_seq_kernel_client_dispatch(int client, struct snd_seq_event * ev,
EXPORT_SYMBOL(snd_seq_kernel_client_dispatch);
-/*
- exported, called by kernel clients to perform same functions as with
- userland ioctl()
+/**
- snd_seq_kernel_client_ctl - operate a command for a client with data in
kernel space.
- @clientid: A numerical ID for a client.
- @cmd: An ioctl(2) command for ALSA sequencer operation.
- @arg: A pointer to data in kernel space.
- Against its name, both kernel/application client can be handled by this
- kernel API. Address space for 'arg' argument should be in kernel space.
- Return: 0 at success. Minus error code at failure.
s/Minus/Negative/
Takashi
In a 32/64 bit compatibility layer of ALSA sequencer core, data for some ioctls is copied to kernel stack and passed to core operations. Then, address limit of running task is changed because core implementation expected arguments in userspace.
In this case, snd_seq_kernel_client_ctl() is available. This commit replaces with it.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_compat.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/sound/core/seq/seq_compat.c b/sound/core/seq/seq_compat.c index 70d3ddb..6cc7302 100644 --- a/sound/core/seq/seq_compat.c +++ b/sound/core/seq/seq_compat.c @@ -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) { int err = -EFAULT; struct snd_seq_port_info *data; - mm_segment_t fs;
data = kmalloc(sizeof(*data), GFP_KERNEL); if (!data) @@ -60,9 +58,7 @@ static int seq_call_port_info_ioctl(struct snd_seq_client *client, goto error; data->kernel = NULL;
- fs = snd_enter_user(); - err = seq_do_ioctl(client, cmd, data); - snd_leave_user(fs); + err = snd_seq_kernel_client_ctl(clientid, cmd, data); if (err < 0) goto error;
@@ -124,17 +120,22 @@ static long snd_seq_ioctl_compat(struct file *file, unsigned int cmd, unsigned l case SNDRV_SEQ_IOCTL_GET_SUBSCRIPTION: case SNDRV_SEQ_IOCTL_QUERY_NEXT_CLIENT: case SNDRV_SEQ_IOCTL_RUNNING_MODE: - return seq_do_ioctl(client, cmd, argp); + return snd_seq_ioctl(file, cmd, (unsigned long)arg); 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); } return -ENOIOCTLCMD; }
On Sun, 07 Aug 2016 11:48:42 +0200, Takashi Sakamoto wrote:
In a 32/64 bit compatibility layer of ALSA sequencer core, data for some ioctls is copied to kernel stack and passed to core operations. Then, address limit of running task is changed because core implementation expected arguments in userspace.
In this case, snd_seq_kernel_client_ctl() is available. This commit replaces with it.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp
sound/core/seq/seq_compat.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/sound/core/seq/seq_compat.c b/sound/core/seq/seq_compat.c index 70d3ddb..6cc7302 100644 --- a/sound/core/seq/seq_compat.c +++ b/sound/core/seq/seq_compat.c @@ -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) { int err = -EFAULT; struct snd_seq_port_info *data;
mm_segment_t fs;
data = kmalloc(sizeof(*data), GFP_KERNEL); if (!data)
@@ -60,9 +58,7 @@ static int seq_call_port_info_ioctl(struct snd_seq_client *client, goto error; data->kernel = NULL;
- fs = snd_enter_user();
- err = seq_do_ioctl(client, cmd, data);
- snd_leave_user(fs);
- err = snd_seq_kernel_client_ctl(clientid, cmd, data); if (err < 0) goto error;
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.
Takashi
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.
Regards
Takashi Sakamoto
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?
Takashi
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.
Regards
Takashi Sakamoto
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
On Aug 9 2016 21:15, Takashi Iwai wrote:
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.
In seq_call_port_info_ioctl(), no members except for the 'number' are used. So no need to get a pointer to the client data as an argument.
This change makes it simpler for readers to follow code of the function. They have no need to think about the whole data, it's OK just to focus on the numerical ID for client.
Regards
Takashi Sakamoto
On Tue, 09 Aug 2016 14:32:30 +0200, Takashi Sakamoto wrote:
On Aug 9 2016 21:15, Takashi Iwai wrote:
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.
In seq_call_port_info_ioctl(), no members except for the 'number' are used. So no need to get a pointer to the client data as an argument.
This change makes it simpler for readers to follow code of the function. They have no need to think about the whole data, it's OK just to focus on the numerical ID for client.
I don't buy this argument, it's no obvious improvement about readability at all, sorry. Why delegating the object becomes so difficult to follow suddenly from that point? IOW, why passing a client number there is easier to understand? The caller doesn't need to care which field is evaluated. C isn't OOP language, but still most of Linux codes are in such a style.
Unless any obvious code readability improvement is done, usually the merit of the code optimization wins. It even avoids the unnecessary changes, makes easier to follow the change history in git tree in this case. It's often user-friendlier than small frequent changes here and there "just for a taste".
Takashi
In former commit, actual operations for each ioctl is re-designed to get data in kernel space. Therefore, no need to cast pointer in kernel space to user space.
This commit applies optimization to in-kernel path of ioctl.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 36 +++++++++++++----------------------- 1 file changed, 13 insertions(+), 23 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index a09cb84..2002dac 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -2248,23 +2248,6 @@ static const struct seq_ioctl_table { { 0, NULL, 0 }, };
-static int seq_do_ioctl(struct snd_seq_client *client, unsigned int cmd, - void __user *arg) -{ - const struct seq_ioctl_table *p; - - if (! arg) - return -EFAULT; - for (p = ioctl_tables; p->cmd; p++) { - if (p->cmd == cmd) - return p->func(client, arg); - } - pr_debug("ALSA: seq unknown ioctl() 0x%x (type='%c', number=0x%02x)\n", - cmd, _IOC_TYPE(cmd), _IOC_NR(cmd)); - return -ENOTTY; -} - - static long snd_seq_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -2509,16 +2492,23 @@ EXPORT_SYMBOL(snd_seq_kernel_client_dispatch); int snd_seq_kernel_client_ctl(int clientid, unsigned int cmd, void *arg) { struct snd_seq_client *client; - mm_segment_t fs; - int result; + const struct seq_ioctl_table *p;
client = clientptr(clientid); if (client == NULL) return -ENXIO; - fs = snd_enter_user(); - result = seq_do_ioctl(client, cmd, (void __force __user *)arg); - snd_leave_user(fs); - return result; + + for (p = ioctl_tables; p->cmd > 0; ++p) { + if (p->cmd == cmd) + break; + } + if (p->cmd == 0) { + pr_debug("ALSA: seq unknown ioctl() 0x%x (type='%c', number=0x%02x)\n", + cmd, _IOC_TYPE(cmd), _IOC_NR(cmd)); + return -ENOTTY; + } + + return p->func(client, arg); }
EXPORT_SYMBOL(snd_seq_kernel_client_ctl);
Former commits make it useless to change address limit of running task. This commit obsoletes helper functions for it.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 15 --------------- 1 file changed, 15 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 2002dac..a643b4e 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -87,21 +87,6 @@ static int snd_seq_deliver_single_event(struct snd_seq_client *client,
/* */ - -static inline mm_segment_t snd_enter_user(void) -{ - mm_segment_t fs = get_fs(); - set_fs(get_ds()); - return fs; -} - -static inline void snd_leave_user(mm_segment_t fs) -{ - set_fs(fs); -} - -/* - */ static inline unsigned short snd_seq_file_flags(struct file *file) { switch (file->f_mode & (FMODE_READ | FMODE_WRITE)) {
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index a643b4e..5e81d4f 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1114,9 +1114,12 @@ static unsigned int snd_seq_poll(struct file *file, poll_table * wait) /*-----------------------------------------------------*/
-static int seq_ioctl_pversion(struct snd_seq_client *client, void __user *arg) +static int seq_ioctl_pversion(struct snd_seq_client *client, void *arg) { - return put_user(SNDRV_SEQ_VERSION, (int __user *)arg) ? -EFAULT : 0; + int *version = arg; + + *version = SNDRV_SEQ_VERSION; + return 0; }
static int seq_ioctl_client_id(struct snd_seq_client *client, void __user *arg)
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 5e81d4f..123d8a1 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1122,9 +1122,12 @@ static int seq_ioctl_pversion(struct snd_seq_client *client, void *arg) return 0; }
-static int seq_ioctl_client_id(struct snd_seq_client *client, void __user *arg) +static int seq_ioctl_client_id(struct snd_seq_client *client, void *arg) { - return put_user(client->number, (int __user *)arg) ? -EFAULT : 0; + int *number = arg; + + *number = client->number; + return 0; }
/* SYSTEM_INFO ioctl() */
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 123d8a1..cb86190 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1131,22 +1131,19 @@ static int seq_ioctl_client_id(struct snd_seq_client *client, void *arg) }
/* SYSTEM_INFO ioctl() */ -static int seq_ioctl_system_info(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_system_info(struct snd_seq_client *client, void *arg) { - struct snd_seq_system_info info; + struct snd_seq_system_info *info = arg;
- memset(&info, 0, sizeof(info)); + memset(info, 0, sizeof(*info)); /* fill the info fields */ - info.queues = SNDRV_SEQ_MAX_QUEUES; - info.clients = SNDRV_SEQ_MAX_CLIENTS; - info.ports = SNDRV_SEQ_MAX_PORTS; - info.channels = 256; /* fixed limit */ - info.cur_clients = client_usage.cur; - info.cur_queues = snd_seq_queue_get_cur_queues(); + info->queues = SNDRV_SEQ_MAX_QUEUES; + info->clients = SNDRV_SEQ_MAX_CLIENTS; + info->ports = SNDRV_SEQ_MAX_PORTS; + info->channels = 256; /* fixed limit */ + info->cur_clients = client_usage.cur; + info->cur_queues = snd_seq_queue_get_cur_queues();
- if (copy_to_user(arg, &info, sizeof(info))) - return -EFAULT; return 0; }
On Sun, 07 Aug 2016 11:48:47 +0200, Takashi Sakamoto wrote:
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp
While it's OK to split to small patches if you prefer, you don't have to do so. Basically all the rest are doing the same thing (strip copy_*_user() and replace to the pointer accesses), and it's rather boring to read repeated mails.
thanks,
Takashi
On Mon, 08 Aug 2016 09:04:55 +0200, Takashi Iwai wrote:
On Sun, 07 Aug 2016 11:48:47 +0200, Takashi Sakamoto wrote:
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp
While it's OK to split to small patches if you prefer, you don't have to do so. Basically all the rest are doing the same thing (strip copy_*_user() and replace to the pointer accesses), and it's rather boring to read repeated mails.
BTW, I'm afraid that the patch series breaks bisection. We need to consider rearranging the changes if we want to keep bisectionability.
Takashi
On Aug 8 2016 16:50, Takashi Iwai wrote:
On Mon, 08 Aug 2016 09:04:55 +0200, Takashi Iwai wrote:
On Sun, 07 Aug 2016 11:48:47 +0200, Takashi Sakamoto wrote:
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp
While it's OK to split to small patches if you prefer, you don't have to do so. Basically all the rest are doing the same thing (strip copy_*_user() and replace to the pointer accesses), and it's rather boring to read repeated mails.
BTW, I'm afraid that the patch series breaks bisection. We need to consider rearranging the changes if we want to keep bisectionability.
As long as it's possible. But in this case, it's difficult. The relation between ioctl table and each functions is one to N. If we change them in one patch, the size is quite large (and alsa-project.org will skip it to deliver.). It's unavoidable.
Regards
Takashi Sakamoto
On Mon, 08 Aug 2016 14:22:47 +0200, Takashi Sakamoto wrote:
On Aug 8 2016 16:50, Takashi Iwai wrote:
On Mon, 08 Aug 2016 09:04:55 +0200, Takashi Iwai wrote:
On Sun, 07 Aug 2016 11:48:47 +0200, Takashi Sakamoto wrote:
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp
While it's OK to split to small patches if you prefer, you don't have to do so. Basically all the rest are doing the same thing (strip copy_*_user() and replace to the pointer accesses), and it's rather boring to read repeated mails.
BTW, I'm afraid that the patch series breaks bisection. We need to consider rearranging the changes if we want to keep bisectionability.
As long as it's possible. But in this case, it's difficult. The relation between ioctl table and each functions is one to N. If we change them in one patch, the size is quite large (and alsa-project.org will skip it to deliver.). It's unavoidable.
The standard technique is transitional changes: namely, introduce a new function table while keeping both old and new tables, move the entries gradually, and finally clean up the old table.
In your case, try to start cleaning up the user-copy at first. That is,
- Create a function to do user-copy and call function pointer with a new function table. It's called from snd_seq_do_ioctl(), and snd_seq_do_ioctl() processes the old function table as fallback.
- Move each function to the new function.
- After moving all functions, replace snd_seq_do_ioctl() with the new one you created in the first step.
At this moment, snd_seq_kernel_client_ctl() still calls snd_seq_do_ioctl(). Now we can clean up the segment workaround. At best, retrieve a lookup function for obtaining the function pointer from the given ioctl number. Then both snd_seq_kernel_client_ctl() and seq_seq_do_ioctl() can call it and delegate to the function.
At last, clean up the compat code.
Takashi
On Aug 8 2016 16:04, Takashi Iwai wrote:
On Sun, 07 Aug 2016 11:48:47 +0200, Takashi Sakamoto wrote:
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp
While it's OK to split to small patches if you prefer, you don't have to do so. Basically all the rest are doing the same thing (strip copy_*_user() and replace to the pointer accesses), and it's rather boring to read repeated mails.
The mail server of alsa-project.org has a limitation of the size of one message. It shrinks developers to split commit to a small parts. Not from my taste.
Regards
Takashi Sakamoto
On Mon, 08 Aug 2016 10:37:21 +0200, Takashi Sakamoto wrote:
On Aug 8 2016 16:04, Takashi Iwai wrote:
On Sun, 07 Aug 2016 11:48:47 +0200, Takashi Sakamoto wrote:
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp
While it's OK to split to small patches if you prefer, you don't have to do so. Basically all the rest are doing the same thing (strip copy_*_user() and replace to the pointer accesses), and it's rather boring to read repeated mails.
The mail server of alsa-project.org has a limitation of the size of one message. It shrinks developers to split commit to a small parts. Not from my taste.
Yeah, that's sometimes annoying, indeed. Jaroslav, could you raise the bar to allow larger patches, or drop this restriction?
Meanwhile, the current limitation is reasonable in most cases. Patches with thousands of lines make review more difficult. If your patch exceeds the size, it already indicates that it may be potentially a bad patch. The logic is something similar like 80 chars limitation we have in general.
Sometimes a big patch is unavoidable (e.g. writing a new driver from scratch). Or sometimes a bigger patch is acceptable (e.g. when changes are machinery, done purely by a script). But if you wrote some patch by hands and it exceeds the size, you should doubt it at first.
Takashi
On Aug 9 2016 00:07, Takashi Iwai wrote:
On Mon, 08 Aug 2016 10:37:21 +0200, Takashi Sakamoto wrote:
On Aug 8 2016 16:04, Takashi Iwai wrote:
On Sun, 07 Aug 2016 11:48:47 +0200, Takashi Sakamoto wrote:
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp
While it's OK to split to small patches if you prefer, you don't have to do so. Basically all the rest are doing the same thing (strip copy_*_user() and replace to the pointer accesses), and it's rather boring to read repeated mails.
The mail server of alsa-project.org has a limitation of the size of one message. It shrinks developers to split commit to a small parts. Not from my taste.
Yeah, that's sometimes annoying, indeed. Jaroslav, could you raise the bar to allow larger patches, or drop this restriction?
It's not what I want. I've already followed to the restriction for this several years. The restriction is still OK as long as I can split my patches as small parts and I'm not objected for them.
You completely puzzled me. Finally, what you'd like me to do?????
Regards
Takashi Sakamoto
On Tue, 09 Aug 2016 14:15:03 +0200, Takashi Sakamoto wrote:
On Aug 9 2016 00:07, Takashi Iwai wrote:
On Mon, 08 Aug 2016 10:37:21 +0200, Takashi Sakamoto wrote:
On Aug 8 2016 16:04, Takashi Iwai wrote:
On Sun, 07 Aug 2016 11:48:47 +0200, Takashi Sakamoto wrote:
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp
While it's OK to split to small patches if you prefer, you don't have to do so. Basically all the rest are doing the same thing (strip copy_*_user() and replace to the pointer accesses), and it's rather boring to read repeated mails.
The mail server of alsa-project.org has a limitation of the size of one message. It shrinks developers to split commit to a small parts. Not from my taste.
Yeah, that's sometimes annoying, indeed. Jaroslav, could you raise the bar to allow larger patches, or drop this restriction?
It's not what I want. I've already followed to the restriction for this several years. The restriction is still OK as long as I can split my patches as small parts and I'm not objected for them.
You completely puzzled me. Finally, what you'd like me to do?????
You've cut into too small patches. Damn too small.
Of course, too small is much better than too big. That's why I wrote "it's OK". But it's also a bad taste, after all -- it annoys readers, since they need to look over the same boring changes again and again.
For avoiding annoyance, such tasks would be better to be grouped in a certain level, so that readers can take a glance over wider ranges, as long as all the changes are the more or less same procedure.
Takashi
On Aug 9 2016 21:24, Takashi Iwai wrote:
You completely puzzled me. Finally, what you'd like me to do?????
You've cut into too small patches. Damn too small.
Of course, too small is much better than too big. That's why I wrote "it's OK". But it's also a bad taste, after all -- it annoys readers, since they need to look over the same boring changes again and again.
For avoiding annoyance, such tasks would be better to be grouped in a certain level, so that readers can take a glance over wider ranges, as long as all the changes are the more or less same procedure.
And bisect operations have a practical effect.
Hm, OK. I see. Depending on the purpose of patches, I'm likely to create large patches.
Honestly, I have a long concern about the granularity of the size of each patch. For example, change log of the latest alsa-lib in alsa-project.org is quite bad and I was a bit disappointed.
Regards
Takashi Sakamoto
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index cb86190..10a2e43 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1149,38 +1149,34 @@ static int seq_ioctl_system_info(struct snd_seq_client *client, void *arg)
/* RUNNING_MODE ioctl() */ -static int seq_ioctl_running_mode(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_running_mode(struct snd_seq_client *client, void *arg) { - struct snd_seq_running_info info; + struct snd_seq_running_info *info = arg; struct snd_seq_client *cptr; int err = 0;
- if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT; - /* requested client number */ - cptr = snd_seq_client_use_ptr(info.client); + cptr = snd_seq_client_use_ptr(info->client); if (cptr == NULL) return -ENOENT; /* don't change !!! */
#ifdef SNDRV_BIG_ENDIAN - if (! info.big_endian) { + if (!info->big_endian) { err = -EINVAL; goto __err; } #else - if (info.big_endian) { + if (info->big_endian) { err = -EINVAL; goto __err; }
#endif - if (info.cpu_mode > sizeof(long)) { + if (info->cpu_mode > sizeof(long)) { err = -EINVAL; goto __err; } - cptr->convert32 = (info.cpu_mode < sizeof(long)); + cptr->convert32 = (info->cpu_mode < sizeof(long)); __err: snd_seq_client_unlock(cptr); return err;
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 10a2e43..3e7431e 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1209,25 +1209,19 @@ static void get_client_info(struct snd_seq_client *cptr, memset(info->reserved, 0, sizeof(info->reserved)); }
-static int seq_ioctl_get_client_info(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_get_client_info(struct snd_seq_client *client, void *arg) { + struct snd_seq_client_info *client_info = arg; struct snd_seq_client *cptr; - struct snd_seq_client_info client_info; - - if (copy_from_user(&client_info, arg, sizeof(client_info))) - return -EFAULT;
/* requested client number */ - cptr = snd_seq_client_use_ptr(client_info.client); + cptr = snd_seq_client_use_ptr(client_info->client); if (cptr == NULL) return -ENOENT; /* don't change !!! */
- get_client_info(cptr, &client_info); + get_client_info(cptr, client_info); snd_seq_client_unlock(cptr);
- if (copy_to_user(arg, &client_info, sizeof(client_info))) - return -EFAULT; return 0; }
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 3e7431e..46e040b 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1227,28 +1227,24 @@ static int seq_ioctl_get_client_info(struct snd_seq_client *client, void *arg)
/* CLIENT_INFO ioctl() */ -static int seq_ioctl_set_client_info(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_set_client_info(struct snd_seq_client *client, void *arg) { - struct snd_seq_client_info client_info; - - if (copy_from_user(&client_info, arg, sizeof(client_info))) - return -EFAULT; + struct snd_seq_client_info *client_info = arg;
/* it is not allowed to set the info fields for an another client */ - if (client->number != client_info.client) + if (client->number != client_info->client) return -EPERM; /* also client type must be set now */ - if (client->type != client_info.type) + if (client->type != client_info->type) return -EINVAL;
/* fill the info fields */ - if (client_info.name[0]) - strlcpy(client->name, client_info.name, sizeof(client->name)); + if (client_info->name[0]) + strlcpy(client->name, client_info->name, sizeof(client->name));
- client->filter = client_info.filter; - client->event_lost = client_info.event_lost; - memcpy(client->event_filter, client_info.event_filter, 32); + client->filter = client_info->filter; + client->event_lost = client_info->event_lost; + memcpy(client->event_filter, client_info->event_filter, 32);
return 0; }
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 46e040b..c2bb93b 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1253,30 +1253,27 @@ static int seq_ioctl_set_client_info(struct snd_seq_client *client, void *arg) /* * CREATE PORT ioctl() */ -static int seq_ioctl_create_port(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_create_port(struct snd_seq_client *client, void *arg) { + struct snd_seq_port_info *info = arg; struct snd_seq_client_port *port; - struct snd_seq_port_info info; struct snd_seq_port_callback *callback;
- if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT; - /* it is not allowed to create the port for an another client */ - if (info.addr.client != client->number) + if (info->addr.client != client->number) return -EPERM;
- port = snd_seq_create_port(client, (info.flags & SNDRV_SEQ_PORT_FLG_GIVEN_PORT) ? info.addr.port : -1); + port = snd_seq_create_port(client, (info->flags & SNDRV_SEQ_PORT_FLG_GIVEN_PORT) ? info->addr.port : -1); if (port == NULL) return -ENOMEM;
- if (client->type == USER_CLIENT && info.kernel) { + if (client->type == USER_CLIENT && info->kernel) { snd_seq_delete_port(client, port->addr.port); return -EINVAL; } if (client->type == KERNEL_CLIENT) { - if ((callback = info.kernel) != NULL) { + callback = info->kernel; + if (callback != NULL) { if (callback->owner) port->owner = callback->owner; port->private_data = callback->private_data; @@ -1289,14 +1286,11 @@ static int seq_ioctl_create_port(struct snd_seq_client *client, } }
- info.addr = port->addr; + info->addr = port->addr;
- snd_seq_set_port_info(port, &info); + snd_seq_set_port_info(port, info); snd_seq_system_client_ev_port_start(port->addr.client, port->addr.port);
- if (copy_to_user(arg, &info, sizeof(info))) - return -EFAULT; - return 0; }
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index c2bb93b..3acec92 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1297,23 +1297,19 @@ static int seq_ioctl_create_port(struct snd_seq_client *client, void *arg) /* * DELETE PORT ioctl() */ -static int seq_ioctl_delete_port(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_delete_port(struct snd_seq_client *client, void *arg) { - struct snd_seq_port_info info; + struct snd_seq_port_info *info = arg; int err;
- /* set passed parameters */ - if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT; - /* it is not allowed to remove the port for an another client */ - if (info.addr.client != client->number) + if (info->addr.client != client->number) return -EPERM;
- err = snd_seq_delete_port(client, info.addr.port); + err = snd_seq_delete_port(client, info->addr.port); if (err >= 0) - snd_seq_system_client_ev_port_exit(client->number, info.addr.port); + snd_seq_system_client_ev_port_exit(client->number, + info->addr.port); return err; }
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 3acec92..1e7b2ac 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1317,32 +1317,27 @@ static int seq_ioctl_delete_port(struct snd_seq_client *client, void *arg) /* * GET_PORT_INFO ioctl() (on any client) */ -static int seq_ioctl_get_port_info(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_get_port_info(struct snd_seq_client *client, void *arg) { + struct snd_seq_port_info *info = arg; struct snd_seq_client *cptr; struct snd_seq_client_port *port; - struct snd_seq_port_info info;
- if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT; - cptr = snd_seq_client_use_ptr(info.addr.client); + cptr = snd_seq_client_use_ptr(info->addr.client); if (cptr == NULL) return -ENXIO;
- port = snd_seq_port_use_ptr(cptr, info.addr.port); + port = snd_seq_port_use_ptr(cptr, info->addr.port); if (port == NULL) { snd_seq_client_unlock(cptr); return -ENOENT; /* don't change */ }
/* get port info */ - snd_seq_get_port_info(port, &info); + snd_seq_get_port_info(port, info); snd_seq_port_unlock(port); snd_seq_client_unlock(cptr);
- if (copy_to_user(arg, &info, sizeof(info))) - return -EFAULT; return 0; }
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 1e7b2ac..4b4cd66 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1345,20 +1345,16 @@ static int seq_ioctl_get_port_info(struct snd_seq_client *client, void *arg) /* * SET_PORT_INFO ioctl() (only ports on this/own client) */ -static int seq_ioctl_set_port_info(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_set_port_info(struct snd_seq_client *client, void *arg) { + struct snd_seq_port_info *info = arg; struct snd_seq_client_port *port; - struct snd_seq_port_info info; - - if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT;
- if (info.addr.client != client->number) /* only set our own ports ! */ + if (info->addr.client != client->number) /* only set our own ports ! */ return -EPERM; - port = snd_seq_port_use_ptr(client, info.addr.port); + port = snd_seq_port_use_ptr(client, info->addr.port); if (port) { - snd_seq_set_port_info(port, &info); + snd_seq_set_port_info(port, info); snd_seq_port_unlock(port); } return 0;
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 4b4cd66..8681f90 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1419,35 +1419,37 @@ int snd_seq_client_notify_subscription(int client, int port, /* * add to port's subscription list IOCTL interface */ -static int seq_ioctl_subscribe_port(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_subscribe_port(struct snd_seq_client *client, void *arg) { + struct snd_seq_port_subscribe *subs = arg; int result = -EINVAL; struct snd_seq_client *receiver = NULL, *sender = NULL; struct snd_seq_client_port *sport = NULL, *dport = NULL; - struct snd_seq_port_subscribe subs; - - if (copy_from_user(&subs, arg, sizeof(subs))) - return -EFAULT;
- if ((receiver = snd_seq_client_use_ptr(subs.dest.client)) == NULL) + receiver = snd_seq_client_use_ptr(subs->dest.client); + if (receiver == NULL) goto __end; - if ((sender = snd_seq_client_use_ptr(subs.sender.client)) == NULL) + sender = snd_seq_client_use_ptr(subs->sender.client); + if (sender == NULL) goto __end; - if ((sport = snd_seq_port_use_ptr(sender, subs.sender.port)) == NULL) + sport = snd_seq_port_use_ptr(sender, subs->sender.port); + if (sport == NULL) goto __end; - if ((dport = snd_seq_port_use_ptr(receiver, subs.dest.port)) == NULL) + dport = snd_seq_port_use_ptr(receiver, subs->dest.port); + if (dport == NULL) goto __end;
- result = check_subscription_permission(client, sport, dport, &subs); + result = check_subscription_permission(client, sport, dport, subs); if (result < 0) goto __end;
/* connect them */ - result = snd_seq_port_connect(client, sender, sport, receiver, dport, &subs); - if (! result) /* broadcast announce */ - snd_seq_client_notify_subscription(SNDRV_SEQ_ADDRESS_SUBSCRIBERS, 0, - &subs, SNDRV_SEQ_EVENT_PORT_SUBSCRIBED); + result = snd_seq_port_connect(client, sender, sport, receiver, dport, + subs); + if (!result) /* broadcast announce */ + snd_seq_client_notify_subscription( + SNDRV_SEQ_ADDRESS_SUBSCRIBERS, 0, + subs, SNDRV_SEQ_EVENT_PORT_SUBSCRIBED); __end: if (sport) snd_seq_port_unlock(sport);
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 8681f90..9f4bcf2 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1466,34 +1466,36 @@ static int seq_ioctl_subscribe_port(struct snd_seq_client *client, void *arg) /* * remove from port's subscription list */ -static int seq_ioctl_unsubscribe_port(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_unsubscribe_port(struct snd_seq_client *client, void *arg) { + struct snd_seq_port_subscribe *subs = arg; int result = -ENXIO; struct snd_seq_client *receiver = NULL, *sender = NULL; struct snd_seq_client_port *sport = NULL, *dport = NULL; - struct snd_seq_port_subscribe subs; - - if (copy_from_user(&subs, arg, sizeof(subs))) - return -EFAULT;
- if ((receiver = snd_seq_client_use_ptr(subs.dest.client)) == NULL) + receiver = snd_seq_client_use_ptr(subs->dest.client); + if (receiver == NULL) goto __end; - if ((sender = snd_seq_client_use_ptr(subs.sender.client)) == NULL) + sender = snd_seq_client_use_ptr(subs->sender.client); + if (receiver == NULL) goto __end; - if ((sport = snd_seq_port_use_ptr(sender, subs.sender.port)) == NULL) + sport = snd_seq_port_use_ptr(sender, subs->sender.port); + if (sport == NULL) goto __end; - if ((dport = snd_seq_port_use_ptr(receiver, subs.dest.port)) == NULL) + dport = snd_seq_port_use_ptr(receiver, subs->dest.port); + if (dport == NULL) goto __end;
- result = check_subscription_permission(client, sport, dport, &subs); + result = check_subscription_permission(client, sport, dport, subs); if (result < 0) goto __end;
- result = snd_seq_port_disconnect(client, sender, sport, receiver, dport, &subs); - if (! result) /* broadcast announce */ - snd_seq_client_notify_subscription(SNDRV_SEQ_ADDRESS_SUBSCRIBERS, 0, - &subs, SNDRV_SEQ_EVENT_PORT_UNSUBSCRIBED); + result = snd_seq_port_disconnect(client, sender, sport, receiver, dport, + subs); + if (!result) /* broadcast announce */ + snd_seq_client_notify_subscription( + SNDRV_SEQ_ADDRESS_SUBSCRIBERS, 0, subs, + SNDRV_SEQ_EVENT_PORT_UNSUBSCRIBED); __end: if (sport) snd_seq_port_unlock(sport);
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 9f4bcf2..aa937c0 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1510,17 +1510,13 @@ static int seq_ioctl_unsubscribe_port(struct snd_seq_client *client, void *arg)
/* CREATE_QUEUE ioctl() */ -static int seq_ioctl_create_queue(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_create_queue(struct snd_seq_client *client, void *arg) { - struct snd_seq_queue_info info; + struct snd_seq_queue_info *info = arg; int result; struct snd_seq_queue *q;
- if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT; - - result = snd_seq_queue_alloc(client->number, info.locked, info.flags); + result = snd_seq_queue_alloc(client->number, info->locked, info->flags); if (result < 0) return result;
@@ -1528,19 +1524,16 @@ static int seq_ioctl_create_queue(struct snd_seq_client *client, if (q == NULL) return -EINVAL;
- info.queue = q->queue; - info.locked = q->locked; - info.owner = q->owner; + info->queue = q->queue; + info->locked = q->locked; + info->owner = q->owner;
/* set queue name */ - if (! info.name[0]) - snprintf(info.name, sizeof(info.name), "Queue-%d", q->queue); - strlcpy(q->name, info.name, sizeof(q->name)); + if (!info->name[0]) + snprintf(info->name, sizeof(info->name), "Queue-%d", q->queue); + strlcpy(q->name, info->name, sizeof(q->name)); queuefree(q);
- if (copy_to_user(arg, &info, sizeof(info))) - return -EFAULT; - return 0; }
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index aa937c0..167be82 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1538,15 +1538,11 @@ static int seq_ioctl_create_queue(struct snd_seq_client *client, void *arg) }
/* DELETE_QUEUE ioctl() */ -static int seq_ioctl_delete_queue(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_delete_queue(struct snd_seq_client *client, void *arg) { - struct snd_seq_queue_info info; - - if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT; + struct snd_seq_queue_info *info = arg;
- return snd_seq_queue_delete(client->number, info.queue); + return snd_seq_queue_delete(client->number, info->queue); }
/* GET_QUEUE_INFO ioctl() */
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 167be82..5756e2c 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1546,29 +1546,22 @@ static int seq_ioctl_delete_queue(struct snd_seq_client *client, void *arg) }
/* GET_QUEUE_INFO ioctl() */ -static int seq_ioctl_get_queue_info(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_get_queue_info(struct snd_seq_client *client, void *arg) { - struct snd_seq_queue_info info; + struct snd_seq_queue_info *info = arg; struct snd_seq_queue *q;
- if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT; - - q = queueptr(info.queue); + q = queueptr(info->queue); if (q == NULL) return -EINVAL;
- memset(&info, 0, sizeof(info)); - info.queue = q->queue; - info.owner = q->owner; - info.locked = q->locked; - strlcpy(info.name, q->name, sizeof(info.name)); + memset(info, 0, sizeof(*info)); + info->queue = q->queue; + info->owner = q->owner; + info->locked = q->locked; + strlcpy(info->name, q->name, sizeof(info->name)); queuefree(q);
- if (copy_to_user(arg, &info, sizeof(info))) - return -EFAULT; - return 0; }
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 5756e2c..70f75ec 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1566,36 +1566,32 @@ static int seq_ioctl_get_queue_info(struct snd_seq_client *client, void *arg) }
/* SET_QUEUE_INFO ioctl() */ -static int seq_ioctl_set_queue_info(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_set_queue_info(struct snd_seq_client *client, void *arg) { - struct snd_seq_queue_info info; + struct snd_seq_queue_info *info = arg; struct snd_seq_queue *q;
- if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT; - - if (info.owner != client->number) + if (info->owner != client->number) return -EINVAL;
/* change owner/locked permission */ - if (snd_seq_queue_check_access(info.queue, client->number)) { - if (snd_seq_queue_set_owner(info.queue, client->number, info.locked) < 0) + if (snd_seq_queue_check_access(info->queue, client->number)) { + if (snd_seq_queue_set_owner(info->queue, client->number, info->locked) < 0) return -EPERM; - if (info.locked) - snd_seq_queue_use(info.queue, client->number, 1); + if (info->locked) + snd_seq_queue_use(info->queue, client->number, 1); } else { return -EPERM; }
- q = queueptr(info.queue); - if (! q) + q = queueptr(info->queue); + if (!q) return -EINVAL; if (q->owner != client->number) { queuefree(q); return -EPERM; } - strlcpy(q->name, info.name, sizeof(q->name)); + strlcpy(q->name, info->name, sizeof(q->name)); queuefree(q);
return 0;
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 70f75ec..9afb0aa 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1598,26 +1598,19 @@ static int seq_ioctl_set_queue_info(struct snd_seq_client *client, void *arg) }
/* GET_NAMED_QUEUE ioctl() */ -static int seq_ioctl_get_named_queue(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_get_named_queue(struct snd_seq_client *client, void *arg) { - struct snd_seq_queue_info info; + struct snd_seq_queue_info *info = arg; struct snd_seq_queue *q;
- if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT; - - q = snd_seq_queue_find_name(info.name); + q = snd_seq_queue_find_name(info->name); if (q == NULL) return -EINVAL; - info.queue = q->queue; - info.owner = q->owner; - info.locked = q->locked; + info->queue = q->queue; + info->owner = q->owner; + info->locked = q->locked; queuefree(q);
- if (copy_to_user(arg, &info, sizeof(info))) - return -EFAULT; - return 0; }
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 9afb0aa..40346f2 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1615,35 +1615,29 @@ static int seq_ioctl_get_named_queue(struct snd_seq_client *client, void *arg) }
/* GET_QUEUE_STATUS ioctl() */ -static int seq_ioctl_get_queue_status(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_get_queue_status(struct snd_seq_client *client, void *arg) { - struct snd_seq_queue_status status; + struct snd_seq_queue_status *status = arg; struct snd_seq_queue *queue; struct snd_seq_timer *tmr;
- if (copy_from_user(&status, arg, sizeof(status))) - return -EFAULT; - - queue = queueptr(status.queue); + queue = queueptr(status->queue); if (queue == NULL) return -EINVAL; - memset(&status, 0, sizeof(status)); - status.queue = queue->queue; + memset(status, 0, sizeof(*status)); + status->queue = queue->queue; tmr = queue->timer; - status.events = queue->tickq->cells + queue->timeq->cells; + status->events = queue->tickq->cells + queue->timeq->cells;
- status.time = snd_seq_timer_get_cur_time(tmr); - status.tick = snd_seq_timer_get_cur_tick(tmr); + status->time = snd_seq_timer_get_cur_time(tmr); + status->tick = snd_seq_timer_get_cur_tick(tmr);
- status.running = tmr->running; + status->running = tmr->running;
- status.flags = queue->flags; + status->flags = queue->flags; queuefree(queue);
- if (copy_to_user(arg, &status, sizeof(status))) - return -EFAULT; return 0; }
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 40346f2..2338468 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1643,32 +1643,26 @@ static int seq_ioctl_get_queue_status(struct snd_seq_client *client, void *arg)
/* GET_QUEUE_TEMPO ioctl() */ -static int seq_ioctl_get_queue_tempo(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_get_queue_tempo(struct snd_seq_client *client, void *arg) { - struct snd_seq_queue_tempo tempo; + struct snd_seq_queue_tempo *tempo = arg; struct snd_seq_queue *queue; struct snd_seq_timer *tmr;
- if (copy_from_user(&tempo, arg, sizeof(tempo))) - return -EFAULT; - - queue = queueptr(tempo.queue); + queue = queueptr(tempo->queue); if (queue == NULL) return -EINVAL; - memset(&tempo, 0, sizeof(tempo)); - tempo.queue = queue->queue; + memset(tempo, 0, sizeof(*tempo)); + tempo->queue = queue->queue; tmr = queue->timer;
- tempo.tempo = tmr->tempo; - tempo.ppq = tmr->ppq; - tempo.skew_value = tmr->skew; - tempo.skew_base = tmr->skew_base; + tempo->tempo = tmr->tempo; + tempo->ppq = tmr->ppq; + tempo->skew_value = tmr->skew; + tempo->skew_base = tmr->skew_base; queuefree(queue);
- if (copy_to_user(arg, &tempo, sizeof(tempo))) - return -EFAULT; return 0; }
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 2338468..a90a4a3 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1677,16 +1677,12 @@ int snd_seq_set_queue_tempo(int client, struct snd_seq_queue_tempo *tempo)
EXPORT_SYMBOL(snd_seq_set_queue_tempo);
-static int seq_ioctl_set_queue_tempo(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_set_queue_tempo(struct snd_seq_client *client, void *arg) { + struct snd_seq_queue_tempo *tempo = arg; int result; - struct snd_seq_queue_tempo tempo; - - if (copy_from_user(&tempo, arg, sizeof(tempo))) - return -EFAULT;
- result = snd_seq_set_queue_tempo(client->number, &tempo); + result = snd_seq_set_queue_tempo(client->number, tempo); return result < 0 ? result : 0; }
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index a90a4a3..383f5d3 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1688,17 +1688,13 @@ static int seq_ioctl_set_queue_tempo(struct snd_seq_client *client, void *arg)
/* GET_QUEUE_TIMER ioctl() */ -static int seq_ioctl_get_queue_timer(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_get_queue_timer(struct snd_seq_client *client, void *arg) { - struct snd_seq_queue_timer timer; + struct snd_seq_queue_timer *timer = arg; struct snd_seq_queue *queue; struct snd_seq_timer *tmr;
- if (copy_from_user(&timer, arg, sizeof(timer))) - return -EFAULT; - - queue = queueptr(timer.queue); + queue = queueptr(timer->queue); if (queue == NULL) return -EINVAL;
@@ -1707,19 +1703,17 @@ static int seq_ioctl_get_queue_timer(struct snd_seq_client *client, return -ERESTARTSYS; } tmr = queue->timer; - memset(&timer, 0, sizeof(timer)); - timer.queue = queue->queue; + memset(timer, 0, sizeof(*timer)); + timer->queue = queue->queue;
- timer.type = tmr->type; + timer->type = tmr->type; if (tmr->type == SNDRV_SEQ_TIMER_ALSA) { - timer.u.alsa.id = tmr->alsa_id; - timer.u.alsa.resolution = tmr->preferred_resolution; + timer->u.alsa.id = tmr->alsa_id; + timer->u.alsa.resolution = tmr->preferred_resolution; } mutex_unlock(&queue->timer_mutex); queuefree(queue); - if (copy_to_user(arg, &timer, sizeof(timer))) - return -EFAULT; return 0; }
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 383f5d3..667ad23 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1719,23 +1719,19 @@ static int seq_ioctl_get_queue_timer(struct snd_seq_client *client, void *arg)
/* SET_QUEUE_TIMER ioctl() */ -static int seq_ioctl_set_queue_timer(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_set_queue_timer(struct snd_seq_client *client, void *arg) { + struct snd_seq_queue_timer *timer = arg; int result = 0; - struct snd_seq_queue_timer timer; - - if (copy_from_user(&timer, arg, sizeof(timer))) - return -EFAULT;
- if (timer.type != SNDRV_SEQ_TIMER_ALSA) + if (timer->type != SNDRV_SEQ_TIMER_ALSA) return -EINVAL;
- if (snd_seq_queue_check_access(timer.queue, client->number)) { + if (snd_seq_queue_check_access(timer->queue, client->number)) { struct snd_seq_queue *q; struct snd_seq_timer *tmr;
- q = queueptr(timer.queue); + q = queueptr(timer->queue); if (q == NULL) return -ENXIO; if (mutex_lock_interruptible(&q->timer_mutex)) { @@ -1743,13 +1739,13 @@ static int seq_ioctl_set_queue_timer(struct snd_seq_client *client, return -ERESTARTSYS; } tmr = q->timer; - snd_seq_queue_timer_close(timer.queue); - tmr->type = timer.type; + snd_seq_queue_timer_close(timer->queue); + tmr->type = timer->type; if (tmr->type == SNDRV_SEQ_TIMER_ALSA) { - tmr->alsa_id = timer.u.alsa.id; - tmr->preferred_resolution = timer.u.alsa.resolution; + tmr->alsa_id = timer->u.alsa.id; + tmr->preferred_resolution = timer->u.alsa.resolution; } - result = snd_seq_queue_timer_open(timer.queue); + result = snd_seq_queue_timer_open(timer->queue); mutex_unlock(&q->timer_mutex); queuefree(q); } else {
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 667ad23..30aac28 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1757,23 +1757,17 @@ static int seq_ioctl_set_queue_timer(struct snd_seq_client *client, void *arg)
/* GET_QUEUE_CLIENT ioctl() */ -static int seq_ioctl_get_queue_client(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_get_queue_client(struct snd_seq_client *client, void *arg) { - struct snd_seq_queue_client info; + struct snd_seq_queue_client *info = arg; int used;
- if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT; - - used = snd_seq_queue_is_used(info.queue, client->number); + used = snd_seq_queue_is_used(info->queue, client->number); if (used < 0) return -EINVAL; - info.used = used; - info.client = client->number; + info->used = used; + info->client = client->number;
- if (copy_to_user(arg, &info, sizeof(info))) - return -EFAULT; return 0; }
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 30aac28..6c1b406 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1773,17 +1773,14 @@ static int seq_ioctl_get_queue_client(struct snd_seq_client *client, void *arg)
/* SET_QUEUE_CLIENT ioctl() */ -static int seq_ioctl_set_queue_client(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_set_queue_client(struct snd_seq_client *client, void *arg) { + struct snd_seq_queue_client *info = arg; int err; - struct snd_seq_queue_client info; - - if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT;
- if (info.used >= 0) { - err = snd_seq_queue_use(info.queue, client->number, info.used); + if (info->used >= 0) { + err = snd_seq_queue_use(info->queue, client->number, + info->used); if (err < 0) return err; }
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 34 ++++++++++++++-------------------- 1 file changed, 14 insertions(+), 20 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 6c1b406..2e63082 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1790,37 +1790,31 @@ static int seq_ioctl_set_queue_client(struct snd_seq_client *client, void *arg)
/* GET_CLIENT_POOL ioctl() */ -static int seq_ioctl_get_client_pool(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_get_client_pool(struct snd_seq_client *client, void *arg) { - struct snd_seq_client_pool info; + struct snd_seq_client_pool *info = arg; struct snd_seq_client *cptr;
- if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT; - - cptr = snd_seq_client_use_ptr(info.client); + cptr = snd_seq_client_use_ptr(info->client); if (cptr == NULL) return -ENOENT; - memset(&info, 0, sizeof(info)); - info.client = cptr->number; - info.output_pool = cptr->pool->size; - info.output_room = cptr->pool->room; - info.output_free = info.output_pool; - info.output_free = snd_seq_unused_cells(cptr->pool); + memset(info, 0, sizeof(*info)); + info->client = cptr->number; + info->output_pool = cptr->pool->size; + info->output_room = cptr->pool->room; + info->output_free = info->output_pool; + info->output_free = snd_seq_unused_cells(cptr->pool); if (cptr->type == USER_CLIENT) { - info.input_pool = cptr->data.user.fifo_pool_size; - info.input_free = info.input_pool; + info->input_pool = cptr->data.user.fifo_pool_size; + info->input_free = info->input_pool; if (cptr->data.user.fifo) - info.input_free = snd_seq_unused_cells(cptr->data.user.fifo->pool); + info->input_free = snd_seq_unused_cells(cptr->data.user.fifo->pool); } else { - info.input_pool = 0; - info.input_free = 0; + info->input_pool = 0; + info->input_free = 0; } snd_seq_client_unlock(cptr); - if (copy_to_user(arg, &info, sizeof(info))) - return -EFAULT; return 0; }
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 2e63082..bf86ca3 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1819,44 +1819,42 @@ static int seq_ioctl_get_client_pool(struct snd_seq_client *client, void *arg) }
/* SET_CLIENT_POOL ioctl() */ -static int seq_ioctl_set_client_pool(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_set_client_pool(struct snd_seq_client *client, void *arg) { - struct snd_seq_client_pool info; + struct snd_seq_client_pool *info = arg; int rc;
- if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT; - - if (client->number != info.client) + if (client->number != info->client) return -EINVAL; /* can't change other clients */
- if (info.output_pool >= 1 && info.output_pool <= SNDRV_SEQ_MAX_EVENTS && - (! snd_seq_write_pool_allocated(client) || - info.output_pool != client->pool->size)) { + if (info->output_pool >= 1 && + info->output_pool <= SNDRV_SEQ_MAX_EVENTS && + (!snd_seq_write_pool_allocated(client) || + info->output_pool != client->pool->size)) { if (snd_seq_write_pool_allocated(client)) { /* remove all existing cells */ snd_seq_queue_client_leave_cells(client->number); snd_seq_pool_done(client->pool); } - client->pool->size = info.output_pool; + client->pool->size = info->output_pool; rc = snd_seq_pool_init(client->pool); if (rc < 0) return rc; } if (client->type == USER_CLIENT && client->data.user.fifo != NULL && - info.input_pool >= 1 && - info.input_pool <= SNDRV_SEQ_MAX_CLIENT_EVENTS && - info.input_pool != client->data.user.fifo_pool_size) { + info->input_pool >= 1 && + info->input_pool <= SNDRV_SEQ_MAX_CLIENT_EVENTS && + info->input_pool != client->data.user.fifo_pool_size) { /* change pool size */ - rc = snd_seq_fifo_resize(client->data.user.fifo, info.input_pool); + rc = snd_seq_fifo_resize(client->data.user.fifo, + info->input_pool); if (rc < 0) return rc; - client->data.user.fifo_pool_size = info.input_pool; + client->data.user.fifo_pool_size = info->input_pool; } - if (info.output_room >= 1 && - info.output_room <= client->pool->size) { - client->pool->room = info.output_room; + if (info->output_room >= 1 && + info->output_room <= client->pool->size) { + client->pool->room = info->output_room; }
return seq_ioctl_get_client_pool(client, arg);
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index bf86ca3..b608521 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1862,18 +1862,14 @@ static int seq_ioctl_set_client_pool(struct snd_seq_client *client, void *arg)
/* REMOVE_EVENTS ioctl() */ -static int seq_ioctl_remove_events(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_remove_events(struct snd_seq_client *client, void *arg) { - struct snd_seq_remove_events info; - - if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT; + struct snd_seq_remove_events *info = arg;
/* * Input mostly not implemented XXX. */ - if (info.remove_mode & SNDRV_SEQ_REMOVE_INPUT) { + if (info->remove_mode & SNDRV_SEQ_REMOVE_INPUT) { /* * No restrictions so for a user client we can clear * the whole fifo @@ -1882,8 +1878,8 @@ static int seq_ioctl_remove_events(struct snd_seq_client *client, snd_seq_fifo_clear(client->data.user.fifo); }
- if (info.remove_mode & SNDRV_SEQ_REMOVE_OUTPUT) - snd_seq_queue_remove_cells(client->number, &info); + if (info->remove_mode & SNDRV_SEQ_REMOVE_OUTPUT) + snd_seq_queue_remove_cells(client->number, info);
return 0; }
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index b608521..cd4073d 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1888,27 +1888,25 @@ static int seq_ioctl_remove_events(struct snd_seq_client *client, void *arg) /* * get subscription info */ -static int seq_ioctl_get_subscription(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_get_subscription(struct snd_seq_client *client, void *arg) { + struct snd_seq_port_subscribe *subs = arg; int result; struct snd_seq_client *sender = NULL; struct snd_seq_client_port *sport = NULL; - struct snd_seq_port_subscribe subs; struct snd_seq_subscribers *p;
- if (copy_from_user(&subs, arg, sizeof(subs))) - return -EFAULT; - result = -EINVAL; - if ((sender = snd_seq_client_use_ptr(subs.sender.client)) == NULL) + sender = snd_seq_client_use_ptr(subs->sender.client); + if (sender == NULL) goto __end; - if ((sport = snd_seq_port_use_ptr(sender, subs.sender.port)) == NULL) + sport = snd_seq_port_use_ptr(sender, subs->sender.port); + if (sport == NULL) goto __end; - p = snd_seq_port_get_subscription(&sport->c_src, &subs.dest); + p = snd_seq_port_get_subscription(&sport->c_src, &subs->dest); if (p) { result = 0; - subs = p->info; + *subs = p->info; } else result = -ENOENT;
@@ -1917,10 +1915,7 @@ static int seq_ioctl_get_subscription(struct snd_seq_client *client, snd_seq_port_unlock(sport); if (sender) snd_seq_client_unlock(sender); - if (result >= 0) { - if (copy_to_user(arg, &subs, sizeof(subs))) - return -EFAULT; - } + return result; }
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index cd4073d..e4a9754 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1923,25 +1923,24 @@ static int seq_ioctl_get_subscription(struct snd_seq_client *client, void *arg) /* * get subscription info - check only its presence */ -static int seq_ioctl_query_subs(struct snd_seq_client *client, void __user *arg) +static int seq_ioctl_query_subs(struct snd_seq_client *client, void *arg) { + struct snd_seq_query_subs *subs = arg; int result = -ENXIO; struct snd_seq_client *cptr = NULL; struct snd_seq_client_port *port = NULL; - struct snd_seq_query_subs subs; struct snd_seq_port_subs_info *group; struct list_head *p; int i;
- if (copy_from_user(&subs, arg, sizeof(subs))) - return -EFAULT; - - if ((cptr = snd_seq_client_use_ptr(subs.root.client)) == NULL) + cptr = snd_seq_client_use_ptr(subs->root.client); + if (cptr == NULL) goto __end; - if ((port = snd_seq_port_use_ptr(cptr, subs.root.port)) == NULL) + port = snd_seq_port_use_ptr(cptr, subs->root.port); + if (cptr == NULL) goto __end;
- switch (subs.type) { + switch (subs->type) { case SNDRV_SEQ_QUERY_SUBS_READ: group = &port->c_src; break; @@ -1954,22 +1953,22 @@ static int seq_ioctl_query_subs(struct snd_seq_client *client, void __user *arg)
down_read(&group->list_mutex); /* search for the subscriber */ - subs.num_subs = group->count; + subs->num_subs = group->count; i = 0; result = -ENOENT; list_for_each(p, &group->list_head) { - if (i++ == subs.index) { + if (i++ == subs->index) { /* found! */ struct snd_seq_subscribers *s; - if (subs.type == SNDRV_SEQ_QUERY_SUBS_READ) { + if (subs->type == SNDRV_SEQ_QUERY_SUBS_READ) { s = list_entry(p, struct snd_seq_subscribers, src_list); - subs.addr = s->info.dest; + subs->addr = s->info.dest; } else { s = list_entry(p, struct snd_seq_subscribers, dest_list); - subs.addr = s->info.sender; + subs->addr = s->info.sender; } - subs.flags = s->info.flags; - subs.queue = s->info.queue; + subs->flags = s->info.flags; + subs->queue = s->info.queue; result = 0; break; } @@ -1981,10 +1980,7 @@ static int seq_ioctl_query_subs(struct snd_seq_client *client, void __user *arg) snd_seq_port_unlock(port); if (cptr) snd_seq_client_unlock(cptr); - if (result >= 0) { - if (copy_to_user(arg, &subs, sizeof(subs))) - return -EFAULT; - } + return result; }
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index e4a9754..0f840ca 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1988,32 +1988,26 @@ static int seq_ioctl_query_subs(struct snd_seq_client *client, void *arg) /* * query next client */ -static int seq_ioctl_query_next_client(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_query_next_client(struct snd_seq_client *client, void *arg) { + struct snd_seq_client_info *info = arg; struct snd_seq_client *cptr = NULL; - struct snd_seq_client_info info; - - if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT;
/* search for next client */ - info.client++; - if (info.client < 0) - info.client = 0; - for (; info.client < SNDRV_SEQ_MAX_CLIENTS; info.client++) { - cptr = snd_seq_client_use_ptr(info.client); + info->client++; + if (info->client < 0) + info->client = 0; + for (; info->client < SNDRV_SEQ_MAX_CLIENTS; info->client++) { + cptr = snd_seq_client_use_ptr(info->client); if (cptr) break; /* found */ } if (cptr == NULL) return -ENOENT;
- get_client_info(cptr, &info); + get_client_info(cptr, info); snd_seq_client_unlock(cptr);
- if (copy_to_user(arg, &info, sizeof(info))) - return -EFAULT; return 0; }
In former commit, actual operations of each ioctl command get argument in kernel space. Copying from/to user space is performed outside of the function.
This commit optimizes to the new design.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 0f840ca..6bc29b5 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -2014,35 +2014,30 @@ static int seq_ioctl_query_next_client(struct snd_seq_client *client, void *arg) /* * query next port */ -static int seq_ioctl_query_next_port(struct snd_seq_client *client, - void __user *arg) +static int seq_ioctl_query_next_port(struct snd_seq_client *client, void *arg) { + struct snd_seq_port_info *info = arg; struct snd_seq_client *cptr; struct snd_seq_client_port *port = NULL; - struct snd_seq_port_info info;
- if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT; - cptr = snd_seq_client_use_ptr(info.addr.client); + cptr = snd_seq_client_use_ptr(info->addr.client); if (cptr == NULL) return -ENXIO;
/* search for next port */ - info.addr.port++; - port = snd_seq_port_query_nearest(cptr, &info); + info->addr.port++; + port = snd_seq_port_query_nearest(cptr, info); if (port == NULL) { snd_seq_client_unlock(cptr); return -ENOENT; }
/* get port info */ - info.addr = port->addr; - snd_seq_get_port_info(port, &info); + info->addr = port->addr; + snd_seq_get_port_info(port, info); snd_seq_port_unlock(port); snd_seq_client_unlock(cptr);
- if (copy_to_user(arg, &info, sizeof(info))) - return -EFAULT; return 0; }
participants (3)
-
Clemens Ladisch -
Takashi Iwai -
Takashi Sakamoto