Re: [PATCH] git-compat-util: avoid failing dir ownership checks if running privileged

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux