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

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

On 5/31/24 11:08, David Hildenbrand wrote:
> On 31.05.24 10:12, Philippe Mathieu-Daudé wrote:
>> On 31/5/24 10:01, David Hildenbrand wrote:
>>> On 31.05.24 09:57, Philippe Mathieu-Daudé wrote:
>>>> Hi Michal,
>>>>
>>>> On 31/5/24 09:28, 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.
>>>>
>>>> Watch out, Linux:
>>>>
>>>>      RETURN VALUE
>>>>
>>>>         On success, posix_madvise() returns 0.  On failure,
>>>>         it returns a positive error number.
>>>>
>>>> but on Darwin:
>>>>
>>>>      RETURN VALUES
>>>>
>>>>         Upon successful completion, a value of 0 is returned.
>>>>         Otherwise, a value of -1 is returned and errno is set
>>>>         to indicate the error.
>>>>
>>>> (Haven't checked other POSIX OSes).
>>>>
>>>
>>> ... but it's supposed to follow the "posix standard" :D Maybe an issue
>>> in the docs?
>>>
>>> FreeBSD seems to follow the standard: "The posix_madvise() interface is
>>> identical, except it returns an error number on error and does not
>>> modify errno, and is provided for standards conformance."
>>>
>>> Same with OpenBSD: "The posix_madvise() interface has the same effect,
>>> but returns the error value instead of only setting errno."
>>
>> On Darwin, MADVISE(2):
>>
>>      The posix_madvise() behaves same as madvise() except that it uses
>>      values with POSIX_ prefix for the advice system call argument.
>>
>>      The posix_madvise function is part of IEEE 1003.1-2001 and was first
>>      implemented in Mac OS X 10.2.
>>
>> Per IEEE 1003.1-2001
>> (https://pubs.opengroup.org/onlinepubs/009695399/functions/posix_madvise.html):
>>
>>     RETURN VALUE
>>
>>       Upon successful completion, posix_madvise() shall return zero;
>>       otherwise, an error number shall be returned to indicate the error.
>>
>> Note the use of "shall" which is described in RFC2119 as:
>>
>>      This word, or the adjective "RECOMMENDED", mean that there
>>      may exist valid reasons in particular circumstances to ignore a
>>      particular item, but the full implications must be understood and
>>      carefully weighed before choosing a different course.
> 
> Agreed, so we have to be careful.
> 
> I do wonder if there would be the option for an automatic approach: for
> example, detect if the errno was/was not changed. Hm.
> 

Firstly, thanks Philippe for this great catch! I did think that "posix_"
prefix might mean POSIX is followed. Anyway, looks like the common
denominator is: on success 0 returned. And then, on Darwin, errno is set
and -1 is returned. On everything(?) else, a positive value is returned
and errno is left untouched. So I think we can get away with something
like the following:

int rc = posix_madvise();
if (rc) {
  if (rc > 0) {
    errno = rc;
  }
  return -1;
}
return 0;

Plus a comment explaining the difference on Darwin.

Michal