Re: [PATCH] iothread: Simplify expression in qemu_in_iothread()

[Philippe Mathieu-Daudé]

On 8/2/24 15:28, Peter Maydell wrote:
> On Thu, 8 Feb 2024 at 14:22, Kevin Wolf <kwolf@redhat.com> wrote:
> > Am 08.02.2024 um 11:48 hat Philippe Mathieu-Daudé geschrieben:
> > > BTW using the same pattern:
> > >
> > > -- >8 --
> > > diff --git a/hw/nvram/xlnx-zynqmp-efuse.c b/hw/nvram/xlnx-zynqmp-efuse.c
> > > index ec98456e5d..d074762a25 100644
> > > --- a/hw/nvram/xlnx-zynqmp-efuse.c
> > > +++ b/hw/nvram/xlnx-zynqmp-efuse.c
> > > @@ -582,7 +582,7 @@ static uint64_t
> > > zynqmp_efuse_cache_load_prew(RegisterInfo *reg,
> > >
> > >   static uint64_t zynqmp_efuse_wr_lock_prew(RegisterInfo *reg, uint64_t val)
> > >   {
> > > -    return val == 0xDF0D ? 0 : 1;
> > > +    return val != 0xDF0D;
> > >   }
> >
> > Maybe. I would have to know that device to tell if this is really meant
> > as boolean. Or maybe it should be written 0x0 and 0x1 to signify that
> > it's a register value or something.
>
> This is a RegisterAccessinfo pre_write hook. The docs say:
>   * @pre_write: Pre write callback. Passed the value that's to be written,
>   * immediately before the actual write. The returned value is what is written,
>   * giving the handler a chance to modify the written value.
>
> So it is indeed returning a register value, not a boolean flag
> masquerading as a uint64_t.
>
> > > diff --git a/tests/tcg/aarch64/sysregs.c b/tests/tcg/aarch64/sysregs.c
> > > index 301e61d0dd..bdd73bd181 100644
> > > --- a/tests/tcg/aarch64/sysregs.c
> > > +++ b/tests/tcg/aarch64/sysregs.c
> > > @@ -183,5 +183,5 @@ int main(void)
> > >           return 1;
> > >       }
> > >
> > > -    return should_fail_count == 6 ? 0 : 1;
> > > +    return should_fail_count != 6;
> > >   }
> >
> > This one isn't unclear to me, though. This is EXIT_SUCCESS and
> > EXIT_FAILURE, just open-coded. I think making your change would make it
> > only more confusing.
>
> I agree on this one.

Thanks for both analysis and sorry for my confusion,
I was just looking at the pattern without interpreting
each proper use case.