On Tue, 9 Apr 2024 at 14:32, Anastasia Belova <abel...@astralinux.ru> wrote: > > > > 09/04/24 15:02, Peter Maydell пишет: > > On Tue, 9 Apr 2024 at 12:54, Anastasia Belova <abel...@astralinux.ru> wrote: > >> ch->num can reach values up to 31. Add casting to > >> a larger type before performing left shift to > >> prevent integer overflow. > > If ch->num can only reach up to 31, then 1 << ch->num > > is fine, because QEMU can assume that integers are 32 bits, > > and we compile with -fwrapv so there isn't a problem with > > shifting into the sign bit. > > Right, thanks for your comments. > I didn't know about this flag before. It became more clear for me now.
Yep; if you're using a static analyser you probably want to configure it to accept the behaviours that are undefined-in-standard-C and which get defined behaviour with -fwrapv. This code is definitely a bit dubious, though, because ch_enable_mask is a uint64_t, so the intention was clearly to allow up to 64 channels. So I think we should take this patch anyway, with a slightly adjusted commit message. All the soc_dma.c code will probably be removed in the 9.2 release, because it's only used by the OMAP board models which we've just deprecated, so it doesn't seem worth spending too much time on cleaning up the code, but in this case you've already written the patch. I'll put this patch on my list to apply after we've made the 9.0 release and restarted development for 9.1. thanks -- PMM
