Re: [PATCH v2 3/3] t: add tests for safe.directory when running with sudo

[Junio C Hamano <gitster@pobox.com>]

Carlo Arenas <carenas@gmail.com> writes:

> Since I am renaming it anyway as part of this topic with RFC v4, would
> it be a good idea to require both?
>
> I see the "IKNOWHATIAMDOING" not as a normal GIT_TEST flag, but as a
> "here be dragons!" warning, and I later found that I either
> misremembered it being enabled in the CI, or it was dropped with one
> of those refactors we do often there.
>
> My RFC v4 includes a new nice looking GIT_TEST variable as suggested
> by Phillip which I am also enabling in the CI to hopefully make it
> even more clear that this is only meant to run there, but sadly that
> also means that this patch will likely have a conflict when merged
> upwards.

This must build from the older mainteance tracks like maint-2.30, so
let's keep the changes to absolute minimum, especially since that
will become the base for any further usability tweaks (in an earlier
round you suggested to cover "doas", and other changes may want to
be applied but all of them should be deferred to later changes).

I actually think 1/3 and 3/3 are OK.  Are there remaining issues in
these two patches (which only are tests)?

As to 2/3, I think the code is basically already fine, but a
simplification like the following on top would be a good idea.

 * The callers do not care how errno is modified by the call made
   into extract_id_from_env(); we are potentially clobbering errno
   by calling getenv(), lstat(), geteuid(), etc, and we have no
   "preserve errno as the caller had" around them.  So let's lose
   the saved_errno thing.

 * We clear errno before making strtoul() call, so any non-zero
   errno must have happeneed in strtoul(), which includes ERANGE.
   There is no point chekcing the returned value env_id; if it is
   ULONG_MAX but errno is 0, then the SUDO_UID legitimately is
   naming a user whose UID is that special value, and it is not an
   indication of an overflow.

With the change squashed in, [2/3] can have 

Reviewed-by: Junio C Hamano <gitster@pobox.com>

Thanks.

 git-compat-util.h | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git c/git-compat-util.h w/git-compat-util.h
index dfdd3e4f81..43c9cd0b48 100644
--- c/git-compat-util.h
+++ w/git-compat-util.h
@@ -413,14 +413,11 @@ static inline void extract_id_from_env(const char *env, uid_t *id)
 	if (real_uid && *real_uid) {
 		char *endptr;
 		unsigned long env_id;
-		int saved_errno = errno;
 
 		errno = 0;
 		env_id = strtoul(real_uid, &endptr, 10);
-		if (!errno && !*endptr && env_id <= (uid_t)-1)
+		if (!*endptr && !errno)
 			*id = env_id;
-
-		errno = saved_errno;
 	}
 }