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@xxxxxxxxxxxxxxxx> 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.) Thanks, Michael -- Michael Kerrisk Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/ Linux/UNIX System Programming Training: http://man7.org/training/
