On 9/27/21 06:47, Philippe Mathieu-Daudé wrote: > On 9/27/21 04:21, Bin Meng wrote: >> GCC seems to be strict about processing pattern like "!!for & bar".
What GCC version btw? >> 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 <[email protected]> >> --- >> >> 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... > > 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 */ >> > >
