]> git.ipfire.org Git - thirdparty/git.git/commit - environment.c
wrapper.c: add x{un,}setenv(), and use xsetenv() in environment.c
authorÆvar Arnfjörð Bjarmason <avarab@gmail.com>
Tue, 21 Sep 2021 13:12:59 +0000 (15:12 +0200)
committerJunio C Hamano <gitster@pobox.com>
Wed, 22 Sep 2021 20:15:00 +0000 (13:15 -0700)
commit3540c71ea5ddffff6e473249866cbc7abb8ce509
tree728ce1acc1024c5d6007cc9635400df300fa7ff1
parent4c719308ce59dc70e606f910f40801f2c6051b24
wrapper.c: add x{un,}setenv(), and use xsetenv() in environment.c

Add fatal wrappers for setenv() and unsetenv(). In d7ac12b25d3 (Add
set_git_dir() function, 2007-08-01) we started checking its return
value, and since 48988c4d0c3 (set_git_dir: die when setenv() fails,
2018-03-30) we've had set_git_dir_1() die if we couldn't set it.

Let's provide a wrapper for both, this will be useful in many other
places, a subsequent patch will make another use of xsetenv().

The checking of the return value here is over-eager according to
setenv(3) and POSIX. It's documented as returning just -1 or 0, so
perhaps we should be checking -1 explicitly.

Let's just instead die on any non-zero, if our C library is so broken
as to return something else than -1 on error (and perhaps not set
errno?) the worst we'll do is die with a nonsensical errno value, but
we'll want to die in either case.

Let's make these return "void" instead of "int". As far as I can tell
there's no other x*() wrappers that needed to make the decision of
deviating from the signature in the C library, but since their return
value is only used to indicate errors (so we'd die here), we can catch
unreachable code such as

    if (xsetenv(...) < 0)
        [...];

I think it would be OK skip the NULL check of the "name" here for the
calls to die_errno(). Almost all of our setenv() callers are taking a
constant string hardcoded in the source as the first argument, and for
the rest we can probably assume they've done the NULL check
themselves. Even if they didn't, modern C libraries are forgiving
about it (e.g. glibc formatting it as "(null)"), on those that aren't,
well, we were about to die anyway. But let's include the check anyway
for good measure.

1. https://pubs.opengroup.org/onlinepubs/009604499/functions/setenv.html

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
environment.c
git-compat-util.h
wrapper.c