Hi Philippe, On Mon, Sep 27, 2021 at 12:47 PM Philippe Mathieu-Daudé <f4...@amsat.org> wrote: > > On 9/27/21 04:21, Bin Meng wrote: > > GCC seems to be strict about processing pattern like "!!for & bar". > > When 'bar' is not 0 or 1, it complains with -Werror=parentheses: > > > > suggest parentheses around operand of ‘!’ or change ‘&’ to ‘&&’ or ‘!’ to > > ‘~’ [-Werror=parentheses] > > > > Add a () around "foo && bar", which also improves code readability. > > > > Signed-off-by: Bin Meng <bmeng...@gmail.com> > > --- > > > > hw/dma/sifive_pdma.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c > > index b4fd40573a..b8ec7621f3 100644 > > --- a/hw/dma/sifive_pdma.c > > +++ b/hw/dma/sifive_pdma.c > > @@ -243,7 +243,7 @@ static void sifive_pdma_write(void *opaque, hwaddr > > offset, > > offset &= 0xfff; > > switch (offset) { > > case DMA_CONTROL: > > - claimed = !!s->chan[ch].control & CONTROL_CLAIM; > > + claimed = !!(s->chan[ch].control & CONTROL_CLAIM); > > AFAIK in C logical NOT has precedence over bitwise AND, so IIUC > compilers should read the current code as: > > claimed (!!s->chan[ch].control) & CONTROL_CLAIM; > > meaning this patch is doing more than "improve code readability", > this is a logical change and likely a bug fix...
Yes, you are correct. Indeed this is a bug fix. I did not dig into the operator precedence in detail. I will reword this in v2. > > BTW GCC suggestions are: > > claimed (!!s->chan[ch].control) & CONTROL_CLAIM; > > claimed (!!s->chan[ch].control) && CONTROL_CLAIM; > > > > if (!claimed && (value & CONTROL_CLAIM)) { > > /* reset Next* registers */ > > I was using GCC 9.3.0 on Ubuntu 20.04. Regards, Bin