Following to basic principles of C language programing.

Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- amixer/amixer.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/amixer/amixer.c b/amixer/amixer.c index 36c92eb..ff1a927 100644 --- a/amixer/amixer.c +++ b/amixer/amixer.c @@ -1278,13 +1278,16 @@ static int get_enum_item_index(snd_mixer_elem_t *elem, char **ptrp)

static int sset_enum(snd_mixer_elem_t *elem, unsigned int argc, char **argv) { - unsigned int idx, item = 0; + unsigned int idx; + unsigned int item = 0; int check_flag = ignore_error ? 0 : -1; + int ival; + char *ptr;

for (idx = 1; idx < argc; idx++) { - char *ptr = argv[idx]; + ptr = argv[idx]; while (*ptr) { - int ival = get_enum_item_index(elem, &ptr); + ival = get_enum_item_index(elem, &ptr); if (ival < 0) return check_flag; if (snd_mixer_selem_set_enum_item(elem, item++, ival) >= 0)

In processing of 'sset' operation for enumerated elements, while loop is used to skip separators for arguments which shows the value of each channel. But the actual channel is not incremented, thus this sample causes wrong result that D is set to third channel, instead of the last channel.

$ amixer sset enum-element-13,1019 A,B,,D

Furthermore, when the value of first channel is omitted, the command causes 'Invalid command' output.

This commit fixes these bug, by incrementing channel number according to the loop and move the loop to the beginning of outer loop.

Fixes: 1a19ec153850('amixer: Don't set only the first item in sset_enum()') Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- amixer/amixer.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/amixer/amixer.c b/amixer/amixer.c index ff1a927..a3de375 100644 --- a/amixer/amixer.c +++ b/amixer/amixer.c @@ -1279,22 +1279,28 @@ static int get_enum_item_index(snd_mixer_elem_t *elem, char **ptrp) static int sset_enum(snd_mixer_elem_t *elem, unsigned int argc, char **argv) { unsigned int idx; - unsigned int item = 0; + unsigned int chn; int check_flag = ignore_error ? 0 : -1; int ival; char *ptr;

for (idx = 1; idx < argc; idx++) { ptr = argv[idx]; + chn = 0; while (*ptr) { + /* Skip separators between the value of each channel. */ + while (*ptr == ',' || isspace(*ptr)) { + ptr++; + chn++; + } + + /* Get index for given argument as enumerated item. */ ival = get_enum_item_index(elem, &ptr); if (ival < 0) return check_flag; - if (snd_mixer_selem_set_enum_item(elem, item++, ival) >= 0) + + if (snd_mixer_selem_set_enum_item(elem, chn, ival) >= 0) check_flag = 1; - /* skip separators */ - while (*ptr == ',' || isspace(*ptr)) - ptr++; } } return check_flag;

According to kernel code (snd_ctl_elem_init_enum_names() in sound/core/control.c), the maximum length of item name is 63 characters (+ 1 terminator = 64 bytes). But current amixer implementation uses 40 bytes. This causes name truncation and fail to operation.

This commit fixes this bug by expanding the length of local variables.

Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- amixer/amixer.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/amixer/amixer.c b/amixer/amixer.c index a3de375..65ebf20 100644 --- a/amixer/amixer.c +++ b/amixer/amixer.c @@ -812,7 +812,11 @@ static int show_selem(snd_mixer_t *handle, snd_mixer_selem_id_t *id, const char if (snd_mixer_selem_is_enumerated(elem)) { int i, items; unsigned int idx; - char itemname[40]; + /* + * See snd_ctl_elem_init_enum_names() in + * sound/core/control.c. + */ + char itemname[64]; items = snd_mixer_selem_get_enum_items(elem); printf(" Items:"); for (i = 0; i < items; i++) { @@ -1255,7 +1259,9 @@ static int get_enum_item_index(snd_mixer_elem_t *elem, char **ptrp) { char *ptr = *ptrp; int items, i, len; - char name[40]; + + /* See snd_ctl_elem_init_enum_names() in sound/core/control.c. */ + char name[64]; items = snd_mixer_selem_get_enum_items(elem); if (items <= 0) @@ -1264,6 +1270,7 @@ static int get_enum_item_index(snd_mixer_elem_t *elem, char **ptrp) for (i = 0; i < items; i++) { if (snd_mixer_selem_get_enum_item_name(elem, i, sizeof(name)-1, name) < 0) continue; + len = strlen(name); if (! strncmp(name, ptr, len)) { if (! ptr[len] || ptr[len] == ',' || ptr[len] == '\n') {

For better reading and easy understanding.

Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- amixer/amixer.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/amixer/amixer.c b/amixer/amixer.c index 65ebf20..aec8d01 100644 --- a/amixer/amixer.c +++ b/amixer/amixer.c @@ -1271,14 +1271,18 @@ static int get_enum_item_index(snd_mixer_elem_t *elem, char **ptrp) if (snd_mixer_selem_get_enum_item_name(elem, i, sizeof(name)-1, name) < 0) continue;

+ /* Check given strings itself. */ len = strlen(name); - if (! strncmp(name, ptr, len)) { - if (! ptr[len] || ptr[len] == ',' || ptr[len] == '\n') { - ptr += len; - *ptrp = ptr; - return i; - } - } + if (strncmp(name, ptr, len) != 0) + continue; + + /* Lack of separators between channels. */ + if (ptr[len] != '\0' && ptr[len] != ',' && ptr[len] != '\n') + continue; + + /* OK. The string is exactly one of items. */ + *ptrp = ptr + len; + return i; } return -1; } @@ -1294,9 +1298,9 @@ static int sset_enum(snd_mixer_elem_t *elem, unsigned int argc, char **argv) for (idx = 1; idx < argc; idx++) { ptr = argv[idx]; chn = 0; - while (*ptr) { + while (ptr[0] != '\0') { /* Skip separators between the value of each channel. */ - while (*ptr == ',' || isspace(*ptr)) { + while (ptr[0] == ',' || isspace(ptr[0])) { ptr++; chn++; } @@ -1304,8 +1308,12 @@ static int sset_enum(snd_mixer_elem_t *elem, unsigned int argc, char **argv) /* Get index for given argument as enumerated item. */ ival = get_enum_item_index(elem, &ptr); if (ival < 0) - return check_flag; + break;

+ /* + * Stand success flag when at least one channel is + * changed. + */ if (snd_mixer_selem_set_enum_item(elem, chn, ival) >= 0) check_flag = 1; }

The arguments are parsed as the value of each channel for enumerated element in sset_enum() and get_enum_item_index(), the former is a caller and the latter is a callee. Both of them evaluate the string but use different characters for separator. This brings just cofusion to users.

This commit fix this bug, by changing callee's characters according to caller's characters.

Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- amixer/amixer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/amixer/amixer.c b/amixer/amixer.c index aec8d01..fa4bde1 100644 --- a/amixer/amixer.c +++ b/amixer/amixer.c @@ -1276,8 +1276,8 @@ static int get_enum_item_index(snd_mixer_elem_t *elem, char **ptrp) if (strncmp(name, ptr, len) != 0) continue;

- /* Lack of separators between channels. */ - if (ptr[len] != '\0' && ptr[len] != ',' && ptr[len] != '\n') + /* Lack of string terminator and separator between channels. */ + if (ptr[len] != '\0' && ptr[len] != ',' && !isspace(ptr[len])) continue;

/* OK. The string is exactly one of items. */