Re: pivot_root - wrong check on mount(2)

["Alejandro Colomar (man-pages)" <alx.manpages@gmail.com>]

On 11/27/20 9:30 AM, Michael Kerrisk (man-pages) wrote:
> Hi Alex,
> 
> On 11/26/20 1:28 PM, Alejandro Colomar (mailing lists; readonly) wrote:
>>
>>
>> On 11/26/20 10:31 AM, Michael Kerrisk (man-pages) wrote:
>>> Hello Davide,
>>>
>>> On Thu, 26 Nov 2020 at 01:01, Davide Giorgio <davide@giorgiodavide.it> wrote:
>>>>
>>>> Good morning,
>>>>
>>>> reading the pivot_root man page
>>>> (https://man7.org/linux/man-pages/man2/pivot_root.2.html)
>>>> there seems to be an error in the example source program
>>>> "pivot_root_demo.c".
>>>> In particular, there is a wrong check on the return value of mount(2).
>>>> https://man7.org/linux/man-pages/man2/mount.2.html#RETURN_VALUE
>>>>
>>>> The error is in this line
>>>> if (mount(NULL, "/", NULL, MS_REC | MS_PRIVATE, NULL) == 1)
>>>>
>>>> that should be
>>>> if (mount(NULL, "/", NULL, MS_REC | MS_PRIVATE, NULL) == -1)
>>>>
>>>>
>>>> Thank you for your work, kind regards
>>>
>>> Thanks! Fixed!
>>
>> Hi Michael,
>>
>> What about fixing this from a different approach:
>>
>> instead of comparing against -1
>> for functions that either return either 0 or -1,
>> we can include those functions in the greater family of
>> functions that return either 0 or non-zero (error code).
>> I propose comparing against 0:
>>
>> - if (mount(NULL, "/", NULL, MS_REC | MS_PRIVATE, NULL) == 1)
>> + if (mount(NULL, "/", NULL, MS_REC | MS_PRIVATE, NULL) != 0)
>>
>> I consider this to be safer, simpler,
>> and although negligible, also faster.
>>
>> What are your thoughts?
> 
> History and the standards say -1. (Okay, mount(2) is not in 
> POSIX, but the statement is true for syscalls generally.) So, I
> prefer to use -1 (and always do so in my own code.)
> 
> The check "ret != 0" does not work for system calls that 
> return  a nonnegative value on success (e.g., open()).
> 
> The check "ret < 0" does not work for system calls that
> can legitimately return a value less than zero on success.
> (getpriority() is the most notable example, but there
> are one or two other cases also.)
> 
> The check "ret == -1" is clear, and--in a standards
> sense--precise. Though one must still be careful, since, 
> for example, getpriority() can return -1 on success.
> (See the manual page for info on how to fdeal with this.)


Hi Michael,

Hmmm, I understand.

Thanks,

Alex

> 
> Thanks,
> 
> Michael
>