Re: [PATCH -next 2/5] ASoC: fsl_asrc: force cast the asrc_format type

[Shengjiu Wang <shengjiu.wang@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4465 bytes --]

On Tue, Jul 19, 2022 at 8:39 PM David Laight <David.Laight@aculab.com>
wrote:

> grrr... top-posting because outluck is really stupid :-(
>
>
>
> The definition seems to be:
>
> typedef int __bitwise
> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__bitwise>
> snd_pcm_format_t
> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t>;
>
> #define SNDRV_PCM_FORMAT_S8
> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/SNDRV_PCM_FORMAT_S8>
> ((__force <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__force>
> snd_pcm_format_t
> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t>) 0)
>
> #define SNDRV_PCM_FORMAT_U8
> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/SNDRV_PCM_FORMAT_U8>
> ((__force <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__force>
> snd_pcm_format_t
> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t>) 1)
>
> #define SNDRV_PCM_FORMAT_S16_LE
> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/SNDRV_PCM_FORMAT_S16_LE>
> ((__force <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/__force>
> snd_pcm_format_t
> <https://elixir.bootlin.com/linux/v5.19-rc7/C/ident/snd_pcm_format_t>) 2)
>
> ...
>
> (goes away and looks up __bitwIse)
>
>
>
> I think I’d add:
>
> #define snd_pcm_format(val) ((__force snd_pcm_format_t)(val))
>

Where is this definition? Which header file?
Thanks.

Best regards
Wang Shengjiu

> and use that to remove most of the casts.
>
But the ones where you have (u32 *)&xxx are only valid because u32 and int
> are the same size.
>
> That does sort of happen to be true, but someone might look at all the
> values and
>
> decide that u8 is big enough.
>
> After which the code will still compile, but the data areas get corrupted.
>
> So you really need to use a u32 ‘temp’ variable.
>
>
>
> It would all be slightly less problematic if the ‘force’ casts could be
> sparse only
>
> (ie not seen by the compiler) – so the compiler would do the type checking.
>
>
>
>                 David
>
>
>
> *From:* Shengjiu Wang <shengjiu.wang@gmail.com>
> *Sent:* 19 July 2022 12:07
> *To:* David Laight <David.Laight@ACULAB.COM>
> *Cc:* Mark Brown <broonie@kernel.org>; Shengjiu Wang <
> shengjiu.wang@nxp.com>; Xiubo.Lee@gmail.com; festevam@gmail.com;
> nicoleotsuka@gmail.com; lgirdwood@gmail.com; perex@perex.cz;
> tiwai@suse.com; alsa-devel@alsa-project.org; linuxppc-dev@lists.ozlabs.org;
> linux-kernel@vger.kernel.org
> *Subject:* Re: [PATCH -next 2/5] ASoC: fsl_asrc: force cast the
> asrc_format type
>
>
>
>
>
>
>
> On Tue, Jul 19, 2022 at 6:34 PM David Laight <David.Laight@aculab.com>
> wrote:
>
> From: Mark Brown
> > Sent: 19 July 2022 11:17
> >
> > On Tue, Jul 19, 2022 at 10:01:54AM +0000, David Laight wrote:
> > > From: Shengjiu Wang
> >
> > > > - ret = of_property_read_u32(np, "fsl,asrc-format",
> &asrc->asrc_format);
> > > > + ret = of_property_read_u32(np, "fsl,asrc-format", (u32
> *)&asrc->asrc_format);
> >
> > > Ugg, you really shouldn't need to do that.
> > > It means that something is badly wrong somewhere.
> > > Casting pointers to integer types is just asking for a bug.
> >
> > That's casting one pointer type to another pointer type.
>
> It is casting the address of some type to a 'u32 *'.
> This will then be dereferenced by the called function.
> So the original type better be 32 bits.
>
> I'm also guessing that sparse was complaining about endianness?
> It isn't at all clear that these casts actually fix it.
>
> The sparse is complaining about the snd_pcm_format_t cast to u32/int type.
>
>
>
> The code in include/sound/pcm.h also does such __force cast.
>
> #define _SNDRV_PCM_FMTBIT(fmt)          (1ULL << (__force
> int)SNDRV_PCM_FORMAT_##fmt)
>
>
>
> The change I have made does not cause an issue.
>
>
>
> Best regards
>
> Wang shengjiu
>
>
>
> (Mark: You'll be glad to hear that the office aircon is
> broken again - two weeks lead time on the spare part.)
>
>         David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1
> 1PT, UK
> Registration No: 1397386 (Wales)
>
>
>
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1
> 1PT, UK
> Registration No: 1397386 (Wales)
>
> P *Please consider the environment and don't print this e-mail unless you
> really need to*
>

[-- Attachment #2: Type: text/html, Size: 10204 bytes --]