Hi Michel, Even more generic: Most--if not all--functions can be catalogued as one of the following: __________________________ Success | Error -------------------------- A) 0 | non-zero B) 0 | negative (the intersection of A and C) C) non-negative| negative D) non-zero | NULL D) true | false For error checking, I'd use: A) (ret != 0) [or simply (ret)] B) (ret < 0) C) (ret < 0) D) (ret == NULL) [or simply (!ret)] E) (!ret) This way you avoid any magic numbers such as '-1' for the error code, and it's more difficult to introduce bugs. Also, CPUs usually compare faster against zero, AFAIK. Cheers, 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? > > Thanks, > > Alex > >> >> Cheers, >> >> Michael >>
