At Wed, 24 Aug 2011 19:46:13 +0800, Lu Guanqun wrote:
On Wed, Aug 24, 2011 at 04:42:05PM +0800, Takashi Iwai wrote:
diff --git a/include/sound/asound.h b/include/sound/asound.h index 5d6074f..9f5f55d 100644 --- a/include/sound/asound.h +++ b/include/sound/asound.h @@ -813,6 +813,33 @@ struct snd_ctl_elem_info { unsigned char reserved[64-4*sizeof(unsigned short)]; };
+/* The struct used for ELEM_ADD and ELEM_REPLACE ioctls;
- This is is identical with snd_ctl_elem_info, but just owner field is
- different, keeping the number of elements to be created.
- The unused fields are stripped for simplicity.
- */
+struct snd_ctl_elem_add_info {
- struct snd_ctl_elem_id id; /* element ID */
- snd_ctl_elem_type_t type; /* value type */
- unsigned int access; /* value access (bitmask) */
- unsigned int count; /* count of values */
- __kernel_pid_t elem_count; /* # of elements to create (0=1) */
- union {
struct {long min; /* R: minimum value */long max; /* R: maximum value */long step; /* R: step (0 variable) */} integer;struct {long long min; /* R: minimum value */long long max; /* R: maximum value */long long step; /* R: step (0 variable) */} integer64;unsigned char reserved[128];- } value;
- unsigned char reserved[64];
+};
struct snd_ctl_elem_value { struct snd_ctl_elem_id id; /* W: element ID */ unsigned int indirect: 1; /* W: indirect access - obsoleted */ @@ -854,8 +881,8 @@ struct snd_ctl_tlv { #define SNDRV_CTL_IOCTL_ELEM_LOCK _IOW('U', 0x14, struct snd_ctl_elem_id) #define SNDRV_CTL_IOCTL_ELEM_UNLOCK _IOW('U', 0x15, struct snd_ctl_elem_id) #define SNDRV_CTL_IOCTL_SUBSCRIBE_EVENTS _IOWR('U', 0x16, int) -#define SNDRV_CTL_IOCTL_ELEM_ADD _IOWR('U', 0x17, struct snd_ctl_elem_info) -#define SNDRV_CTL_IOCTL_ELEM_REPLACE _IOWR('U', 0x18, struct snd_ctl_elem_info) +#define SNDRV_CTL_IOCTL_ELEM_ADD _IOWR('U', 0x17, struct snd_ctl_elem_add_info) +#define SNDRV_CTL_IOCTL_ELEM_REPLACE _IOWR('U', 0x18, struct snd_ctl_elem_add_info) #define SNDRV_CTL_IOCTL_ELEM_REMOVE _IOWR('U', 0x19, struct snd_ctl_elem_id) #define SNDRV_CTL_IOCTL_TLV_READ _IOWR('U', 0x1a, struct snd_ctl_tlv) #define SNDRV_CTL_IOCTL_TLV_WRITE _IOWR('U', 0x1b, struct snd_ctl_tlv) diff --git a/sound/core/control.c b/sound/core/control.c index dc2a440..15c171d 100644 --- a/sound/core/control.c +++ b/sound/core/control.c @@ -1064,7 +1064,7 @@ static void snd_ctl_elem_user_free(struct snd_kcontrol *kcontrol) }
static int snd_ctl_elem_add(struct snd_ctl_file *file,
struct snd_ctl_elem_info *info, int replace)
struct snd_ctl_elem_add_info *info, int replace){ struct snd_card *card = file->card; struct snd_kcontrol kctl, *_kctl; @@ -1099,7 +1099,7 @@ static int snd_ctl_elem_add(struct snd_ctl_file *file, if (err < 0) return err; memcpy(&kctl.id, &info->id, sizeof(info->id));
- kctl.count = info->owner ? info->owner : 1;
- kctl.count = info->elem_count ? info->elem_count : 1;
Hi Takashi,
I don't how this approach can solve this problem.
There is no problem. You can't copy the data from ELEM_INFO for ELEM_ADD/REPLACE. It was a wrong assumption to reuse the data. Instead, you had to set up the struct field manually. That is, the test procedure was simply wrong.
There's still an implicit assumption here that user space application will clear info->elem_count to zero before it issues the REPLACE ioctl, right?
Yes, the point is that it's no longer same structure. So, you can't copy from the result from ELEM_INFO ioctl any more. Instead, you need to set up the struct field manually.
access |= SNDRV_CTL_ELEM_ACCESS_USER; kctl.info = snd_ctl_elem_user_info; if (access & SNDRV_CTL_ELEM_ACCESS_READ) @@ -1139,7 +1139,7 @@ static int snd_ctl_elem_add(struct snd_ctl_file *file, ue = kzalloc(sizeof(struct user_element) + private_size, GFP_KERNEL); if (ue == NULL) return -ENOMEM;
- ue->info = *info;
- memcpy(&ue->info, info, sizeof(*info));
Should we add some assertion here checking the size of these two structure should be the same?
It could, at most by BUILD_BUG_ON().
thanks,
Takashi