Re: [PATCH 2/8] ALSA: control: obsolete switch statement with const value table

[Takashi Sakamoto <o-takashi@sakamocchi.jp>]

Hi Lars,

On Feb 11 2015 19:51, Lars-Peter Clausen wrote:
> On 02/11/2015 11:37 AM, Takashi Sakamoto wrote:
>> To check parameters from userspace, snd_ctl_elem_add() has switch
>> statement. This statement checks the number of values in a control and
>> the size of each value. These two parameters are limited by
>> struct snd_ctl_elem_value.value and can be replaced with two sets of two
>> dimension array with constant values, without any calculation.
>>
>> This commit obsolete the switch statement with the array.
>>
>> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
>> ---
>>   sound/core/control.c | 57
>> +++++++++++++++++++++++++---------------------------
>>   1 file changed, 27 insertions(+), 30 deletions(-)
>>
>> diff --git a/sound/core/control.c b/sound/core/control.c
>> index ea49abc..f248bde 100644
>> --- a/sound/core/control.c
>> +++ b/sound/core/control.c
>> @@ -1161,6 +1161,23 @@ 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)
>>   {
>> +    /* The capacity of struct snd_ctl_elem_value.value.*/
>> +    const unsigned int value_sizes[] = {
>> +        [SNDRV_CTL_ELEM_TYPE_BOOLEAN]    = sizeof(long),
>> +        [SNDRV_CTL_ELEM_TYPE_INTEGER]    = sizeof(long),
>> +        [SNDRV_CTL_ELEM_TYPE_ENUMERATED] = sizeof(unsigned int),
>> +        [SNDRV_CTL_ELEM_TYPE_BYTES]    = sizeof(unsigned char),
>> +        [SNDRV_CTL_ELEM_TYPE_IEC958]    = sizeof(struct snd_aes_iec958),
>> +        [SNDRV_CTL_ELEM_TYPE_INTEGER64] = sizeof(long long),
>> +    };
>> +    const unsigned int max_value_counts[] = {
>> +        [SNDRV_CTL_ELEM_TYPE_BOOLEAN]    = 128,
>> +        [SNDRV_CTL_ELEM_TYPE_INTEGER]    = 128,
>> +        [SNDRV_CTL_ELEM_TYPE_ENUMERATED] = 128,
>> +        [SNDRV_CTL_ELEM_TYPE_BYTES]    = 512,
>> +        [SNDRV_CTL_ELEM_TYPE_IEC958]    = 1,
>> +        [SNDRV_CTL_ELEM_TYPE_INTEGER64] = 64,
>> +    };
> 
> The tables should probably be static, no need to put them on the stack.

Oops, exactly. I've been satisfied just to add const operator, against
my intension... Thanks for your correction.


Takashi Sakamoto