diff mbox

[2/5] amixer: fix a bug to handle omitted value in parameter for enumerated element

Message ID 1428510659-30393-3-git-send-email-o-takashi@sakamocchi.jp (mailing list archive)
State New, archived
Headers show

Commit Message

Takashi Sakamoto April 8, 2015, 4:30 p.m. UTC
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(-)

Comments

Takashi Iwai April 9, 2015, 6:09 a.m. UTC | #1
At Thu,  9 Apr 2015 01:30:55 +0900,
Takashi Sakamoto wrote:
> 
> 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++;

This should be handled only for commas, not for spaces.  Two spaces
don't mean to skip two channels, usually.  Also, the space at the
beginning should be just discarded.  The trailing spaces should be
seen as a separator.


Takashi

> +			}
> +
> +			/* 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;
> -- 
> 2.1.0
>
diff mbox

Patch

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;