From 450296f305f139ccb363657efcea439afa4e762a Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Thu, 3 Aug 2017 20:20:43 +0900 Subject: [PATCH] ALSA: control: code refactoring TLV ioctl handler In a design of ALSA control core, execution path bifurcates depending on target element. When a set with the target element has a handler, it's called. Else, registered buffer is copied to user space. These two operations are apparently different. In current implementation, they're on the same function with a condition statement. This makes it a bit hard to understand conditions of each case. This commit splits codes for these two cases. Signed-off-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/control.c | 132 +++++++++++++++++++++++++++++-------------- 1 file changed, 91 insertions(+), 41 deletions(-) diff --git a/sound/core/control.c b/sound/core/control.c index 989292fe33c2..92389000f0df 100644 --- a/sound/core/control.c +++ b/sound/core/control.c @@ -1394,65 +1394,115 @@ static int snd_ctl_subscribe_events(struct snd_ctl_file *file, int __user *ptr) return 0; } +static int call_tlv_handler(struct snd_ctl_file *file, int op_flag, + struct snd_kcontrol *kctl, + struct snd_ctl_elem_id *id, + unsigned int __user *buf, unsigned int size) +{ + static const struct { + int op; + int perm; + } pairs[] = { + {SNDRV_CTL_TLV_OP_READ, SNDRV_CTL_ELEM_ACCESS_TLV_READ}, + {SNDRV_CTL_TLV_OP_WRITE, SNDRV_CTL_ELEM_ACCESS_TLV_WRITE}, + {SNDRV_CTL_TLV_OP_CMD, SNDRV_CTL_ELEM_ACCESS_TLV_COMMAND}, + }; + struct snd_kcontrol_volatile *vd = &kctl->vd[snd_ctl_get_ioff(kctl, id)]; + int i; + int err; + + /* Check support of the request for this element. */ + for (i = 0; i < ARRAY_SIZE(pairs); ++i) { + if (op_flag == pairs[i].op && (vd->access & pairs[i].perm)) + break; + } + if (i == ARRAY_SIZE(pairs)) + return -ENXIO; + + if (kctl->tlv.c == NULL) + return -ENXIO; + + /* When locked, this is unavailable. */ + if (vd->owner != NULL && vd->owner != file) + return -EPERM; + + err = kctl->tlv.c(kctl, op_flag, size, buf); + if (err < 0) + return err; + + if (err > 0) + snd_ctl_notify(file->card, SNDRV_CTL_EVENT_MASK_TLV, id); + + return 0; +} + +static int read_tlv_buf(struct snd_kcontrol *kctl, struct snd_ctl_elem_id *id, + unsigned int __user *buf, unsigned int size) +{ + struct snd_kcontrol_volatile *vd = &kctl->vd[snd_ctl_get_ioff(kctl, id)]; + unsigned int len; + + if (!(vd->access & SNDRV_CTL_ELEM_ACCESS_TLV_READ)) + return -ENXIO; + + if (kctl->tlv.p == NULL) + return -ENXIO; + + len = sizeof(unsigned int) * 2 + kctl->tlv.p[1]; + if (size < len) + return -ENOMEM; + + if (copy_to_user(buf, kctl->tlv.p, len)) + return -EFAULT; + + return 0; +} + static int snd_ctl_tlv_ioctl(struct snd_ctl_file *file, - struct snd_ctl_tlv __user *_tlv, + struct snd_ctl_tlv __user *buf, int op_flag) { - struct snd_card *card = file->card; - struct snd_ctl_tlv tlv; + struct snd_ctl_tlv header; + unsigned int *container; + unsigned int container_size; struct snd_kcontrol *kctl; + struct snd_ctl_elem_id id; struct snd_kcontrol_volatile *vd; - unsigned int len; - if (copy_from_user(&tlv, _tlv, sizeof(tlv))) + if (copy_from_user(&header, buf, sizeof(header))) return -EFAULT; - if (tlv.length < sizeof(unsigned int) * 2) + + /* In design of control core, numerical ID starts at 1. */ + if (header.numid == 0) return -EINVAL; - if (!tlv.numid) + + /* At least, container should include type and length fields. */ + if (header.length < sizeof(unsigned int) * 2) return -EINVAL; + container_size = header.length; + container = buf->tlv; - kctl = snd_ctl_find_numid(card, tlv.numid); + kctl = snd_ctl_find_numid(file->card, header.numid); if (kctl == NULL) return -ENOENT; - if (kctl->tlv.p == NULL) - return -ENXIO; - - vd = &kctl->vd[tlv.numid - kctl->id.numid]; - if ((op_flag == SNDRV_CTL_TLV_OP_READ && - (vd->access & SNDRV_CTL_ELEM_ACCESS_TLV_READ) == 0) || - (op_flag == SNDRV_CTL_TLV_OP_WRITE && - (vd->access & SNDRV_CTL_ELEM_ACCESS_TLV_WRITE) == 0) || - (op_flag == SNDRV_CTL_TLV_OP_CMD && - (vd->access & SNDRV_CTL_ELEM_ACCESS_TLV_COMMAND) == 0)) - return -ENXIO; + /* Calculate index of the element in this set. */ + id = kctl->id; + snd_ctl_build_ioff(&id, kctl, header.numid - id.numid); + vd = &kctl->vd[snd_ctl_get_ioff(kctl, &id)]; if (vd->access & SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK) { - int err; - - if (vd->owner != NULL && vd->owner != file) - return -EPERM; - - err = kctl->tlv.c(kctl, op_flag, tlv.length, _tlv->tlv); - if (err < 0) - return err; - if (err > 0) { - struct snd_ctl_elem_id id = kctl->id; - snd_ctl_notify(card, SNDRV_CTL_EVENT_MASK_TLV, &id); - } + return call_tlv_handler(file, op_flag, kctl, &id, container, + container_size); } else { - if (op_flag != SNDRV_CTL_TLV_OP_READ) - return -ENXIO; - - len = kctl->tlv.p[1] + 2 * sizeof(unsigned int); - if (tlv.length < len) - return -ENOMEM; - - if (copy_to_user(_tlv->tlv, kctl->tlv.p, len)) - return -EFAULT; + if (op_flag == SNDRV_CTL_TLV_OP_READ) { + return read_tlv_buf(kctl, &id, container, + container_size); + } } - return 0; + /* Not supported. */ + return -ENXIO; } static long snd_ctl_ioctl(struct file *file, unsigned int cmd, unsigned long arg) -- 2.20.1