Patrick Wildt <patr...@blueri.se> wrote: > I don't know userland very well, so I have a question. In the middle of > 2019 there have been plenty of changes in regards to changing checks of > syscalls from < 0 to a more strict == -1, like this one in isakmpd: > > ---------------------------- > revision 1.26 > date: 2019/06/28 13:32:44; author: deraadt; state: Exp; lines: +2 -2; > commitid: JJ6Ck4WTrgUiEjJp; > When system calls indicate an error they return -1, not some arbitrary > value < 0. errno is only updated in this case. Change all (most?) > callers of syscalls to follow this better, and let's see if this strictness > helps us in the future. > ----------------------------
I have about 4000 more changes like that, but I'm stuck with trying to push it further forward. For various reasons, some of which can be guessed from this thread. > getsockopt(), I think, is also a system call. And the manpage indicates > that a failure is always -1, and not some arbitrary number: > > RETURN VALUES > Upon successful completion, the value 0 is returned; otherwise the > value -1 is returned and the global variable errno is set to indicate the > error. > > What is the difference between the diff in this mail, and the changes > done in the middle of last year? The difference is this is direct checking of the syscalls. Versus checking at a higher layer of abstraction, or conversion of that result to something else. Say you have an interface which returns precisely 0 and -1 for two conditions. Well then it has a large set of out-of-range values which (a) won't occur but (b) if they occur, how do you handle them? At which layer? The range of numbers returned really express 3 conditions. One which is impossible, yet if it happens, do you want to convert the impossible to success, or to failure? In the recently supplied diff, a return value of 50 at the system call layer, is returned into a library returning 0 (success). Furthermore, the diff itself proposes treating the out-of-range impossible as a success, and accesses memory which is very probably unintialized. > getsockopt() isn't allowed to return > anything else but 0 and 1, right? Though I guess the current check > (error != 0) is the one that also catches instances where getsockopt() > isn't behaving well, even though it shouldn't. But then, with the -1 > check, wouldn't we be catching more instances of syscalls misbehaving > if we checked for < -1? Correct. I hope you have reached the same indecision point as me. I feel uncomfortable changing all checking-points to 3-way decision. And imagine what a modern compiler would do there...
