On Wed, Apr 27, 2022 at 9:19 AM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > And that is why it is wrong to call it "error". It is not about > errors, but is about the shape of the input string, i.e. where the > run of digits ends. I called it error, because I only cared about it to signal that the string it converted was an erroneous one (ex: "1a" or "a") > And there is this note: > > Since 0, {LONG_MIN} or {LLONG_MIN}, and {LONG_MAX} or {LLONG_MAX} > are returned on error and are also valid returns on success, an > application wishing to check for error situations should set errno > to 0, then call strtol() or strtoll(), then check errno. > > So, no, I do not think the range check gives us anything > meaningful. the meaningful part of it was meant to be: this function will try to get the value of an UID from SUDO_UID, if it gets anything insane like LONG_MIN or LONG_MAX or anything above or below that, it would consider you are just trying to play a bad joke with it and ignore you. > At this point, it is tempting to say that it is not worth trying to > do it right by avoiding atoi(), as we are bound to make more of this > kind of mistakes X-<. But at the same time, I hope we can learn > together and get this right ;-). except it wasn't a mistake, but maybe poorly documented since I was already going over my quota of added lines for what should have been a 3 line patch. apologies for making the code so cryptic and thanks for spending so much time reviewing it and commenting on it, has been definitely very instructive so far. any specific suggestions you would like to have in a reroll? Carlo