Re: [PATCH v3 1/4] osdep: Make qemu_madvise() to set errno in all cases

["Michal Prívozník" <mprivozn@redhat.com>]

On 5/31/24 17:46, Philippe Mathieu-Daudé wrote:
> On 31/5/24 17:10, Michal Privoznik wrote:
>> The unspoken premise of qemu_madvise() is that errno is set on
>> error. And it is mostly the case except for posix_madvise() which
>> is documented to return either zero (on success) or a positive
>> error number. This means, we must set errno ourselves. And while
>> at it, make the function return a negative value on error, just
>> like other error paths do.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>   util/osdep.c | 14 +++++++++++++-
>>   1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/util/osdep.c b/util/osdep.c
>> index e996c4744a..1345238a5c 100644
>> --- a/util/osdep.c
>> +++ b/util/osdep.c
>> @@ -57,7 +57,19 @@ int qemu_madvise(void *addr, size_t len, int advice)
>>   #if defined(CONFIG_MADVISE)
>>       return madvise(addr, len, advice);
>>   #elif defined(CONFIG_POSIX_MADVISE)
>> -    return posix_madvise(addr, len, advice);
>> +    /*
>> +     * On Darwin posix_madvise() has the same return semantics as
>> +     * plain madvise, i.e. errno is set and -1 is returned. Otherwise,
>> +     * a positive error number is returned.
>> +     */
> 
> Alternative is to guard with #ifdef CONFIG_DARWIN ... #else ... #endif
> which might be clearer.

That's how I had it written (locally) initially, but then thought: well,
what if there's another OS that behaves the same? This way, we don't
have to care and just do the right thing.

> 
> Although this approach seems reasonable, so:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Thanks!

Michal