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

[Richard Henderson <richard.henderson@linaro.org>]

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~