Re: [PATCH v2 1/3] include/exec: Add WITH_MMAP_LOCK_GUARD

[Richard Henderson]

On 7/23/23 15:18, Peter Maydell wrote:
> On Sat, 22 Jul 2023 at 12:35, Richard Henderson
> <richard.henderson@linaro.org> wrote:
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > ---
> >   include/exec/exec-all.h | 10 ++++++++++
> >   bsd-user/mmap.c         |  1 +
> >   linux-user/mmap.c       |  1 +
> >   3 files changed, 12 insertions(+)
> >
> > diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> > index 5fa0687cd2..d02517e95f 100644
> > --- a/include/exec/exec-all.h
> > +++ b/include/exec/exec-all.h
> > @@ -629,6 +629,15 @@ void TSA_NO_TSA mmap_lock(void);
> >   void TSA_NO_TSA mmap_unlock(void);
> >   bool have_mmap_lock(void);
> >
> > +static inline void mmap_unlock_guard(void *unused)
> > +{
> > +    mmap_unlock();
> > +}
> > +
> > +#define WITH_MMAP_LOCK_GUARD()                                            \
> > +    for (int _mmap_lock_iter __attribute__((cleanup(mmap_unlock_guard)))  \
> > +         = (mmap_lock(), 0); _mmap_lock_iter == 0; _mmap_lock_iter = 1)
>
> All our other WITH_FOO macros seem to use g_autoptr rather than
> a raw attribute((cleanup)); is it worth being consistent?

I didn't think it worthwhile, no, since that requires even more boilerplate.

> (This one also doesn't allow nested uses, I think.)

It does, since each variable will shadow the next within each context.


r~