On Tue, 9 Apr 2024 at 14:38, Peter Maydell <peter.mayd...@linaro.org> wrote: > > 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.
Now applied to target-arm.next for 9.1 (with adjustments to the commit message); thanks. -- PMM
