[alsa-devel] [PATCH 4/5] amixer: arrange validation logic

[Takashi Iwai]

At Thu,  9 Apr 2015 01:30:57 +0900,
Takashi Sakamoto wrote:
> 
> For better reading and easy understanding.
> 
> Signed-off-by: Takashi Sakamoto <o-takashi at 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;

In general, the negation makes the logic harder to follow.
That is, a code like below (it's almost same as the original one) is
easier, more understandable.

		/* EOS or separator? */
		if (ptr[len] || ptr[len] == ',' || ptr[len] == '\n') {
			*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') {

ptr[0] is less common expression than *ptr, IMO.


Takashi

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