linux-kernel - Re: [greybus-dev] [PATCH 3/3] [PATCH] staging: greybus: __u8 is sufficient for snd_ctl_elem_type_t and snd_ctl_elem_iface_t

Open Source and information security mailing list archives

Message-ID: <160c222d-79e4-c5f0-344f-1a69821db039@linaro.org>
Date:   Fri, 25 Sep 2020 11:02:23 -0500
From:   Alex Elder <elder@...aro.org>
To:     Coiby Xu <coiby.xu@...il.com>,
        David Laight <David.Laight@...lab.com>
Cc:     "devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
        "moderated list:SOUND" <alsa-devel@...a-project.org>,
        Alex Elder <elder@...nel.org>, Takashi Iwai <tiwai@...e.com>,
        Johan Hovold <johan@...nel.org>,
        "moderated list:GREYBUS SUBSYSTEM" <greybus-dev@...ts.linaro.org>,
        Jaroslav Kysela <perex@...ex.cz>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [greybus-dev] [PATCH 3/3] [PATCH] staging: greybus: __u8 is
 sufficient for snd_ctl_elem_type_t and snd_ctl_elem_iface_t

On 9/25/20 9:11 AM, Coiby Xu wrote:
> On Thu, Sep 24, 2020 at 10:54:50AM +0000, David Laight wrote:
>> From: Coiby Xu
>>> Sent: 24 September 2020 11:21
>>> Use __8 to replace int and remove the unnecessary __bitwise type 
>>> attribute.
>>>
>>> Found by sparse,
>> ...
>>> diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
>>> index 535a7229e1d9..8e71a95644ab 100644
>>> --- a/include/uapi/sound/asound.h
>>> +++ b/include/uapi/sound/asound.h
>>> @@ -950,7 +950,7 @@ struct snd_ctl_card_info {
>>>      unsigned char components[128];    /* card components / fine 
>>> identification, delimited with one
>>> space (AC97 etc..) */
>>>  };
>>>
>>> -typedef int __bitwise snd_ctl_elem_type_t;
>>> +typedef __u8 snd_ctl_elem_type_t;
>>>  #define    SNDRV_CTL_ELEM_TYPE_NONE    ((__force 
>>> snd_ctl_elem_type_t) 0) /* invalid */
>>>  #define    SNDRV_CTL_ELEM_TYPE_BOOLEAN    ((__force 
>>> snd_ctl_elem_type_t) 1) /* boolean type */
>>>  #define    SNDRV_CTL_ELEM_TYPE_INTEGER    ((__force 
>>> snd_ctl_elem_type_t) 2) /* integer type */
>>
>> WTF is all that about anyway??
>> What is wrong with:
>> #define    SNDRV_CTL_ELEM_TYPE_NONE    0u /* invalid */
> 
> I'm sorry I don't quite understand you. Are you suggesting 
> SNDRV_CTL_ELEM_TYPE_NONE
> isn't needed in the first place?

I think David is asking why it's defined the way it is,
and I'd guess it's to have the compiler issue an error
if you attempt to assign one of these values to a variable
or field of the wrong type.

No, you should not attempt to change this.

					-Alex
>>     David
>>
>> -
>> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, 
>> MK1 1PT, UK
>> Registration No: 1397386 (Wales)
>>
> 
> -- 
> Best regards,
> Coiby
> _______________________________________________
> greybus-dev mailing list
> greybus-dev@...ts.linaro.org
> https://lists.linaro.org/mailman/listinfo/greybus-dev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives