Re: [PATCH] wrapper: remove xunsetenv()

From: Jeff King <peff@peff.net>
Date: Oct 29, 2021, 21:51 UTC
Message-ID: <YXxs4NZmpjhRkoYA@coredump.intra.peff.net>
In-Reply-To: <YXxpjLhiguq4HY6g@coredump.intra.peff.net>

On Fri, Oct 29, 2021 at 05:37:16PM -0400, Jeff King wrote:

Show 18 quoted lines

> On Fri, Oct 29, 2021 at 02:27:05PM -0700, Carlo Marcelo Arenas Belón wrote:
> 
> > Platforms that are using the git compatibility layer for unsetenv
> > use void as a return value for unsetenv(), so any function that checks
> > for a return value will fail to build.
> 
> Good catch.
> 
> > Remove the unused wrapper function.
> 
> I don't mind removing this if nobody is using it, but doesn't your first
> paragraph argue that our definition of gitunsetenv() is just wrong?
> I.e., it should return an int, even if it is always "0"?
> 
> Or is it a portability question? I.e., are there platforms where
> unsetenv() also returns void, in which case we must make sure nobody
> ever looks at its return value (and xunsetenv() is therefore a wrong
> direction)?

Looks like Junio just posted such a patch in the other thread.

However, according to the unsetenv() manpage:

  Prior to glibc 2.2.2, unsetenv() was prototyped as returning void;
  more recent glibc versions follow the POSIX.1-compliant prototype
  shown in the SYNOPSIS.

So it is POSIX to return an int, but that gives us at least one platform where unsetenv() returns void (or used to). glibc 2.2.2 is 2001-era, so that may be old enough that we don't care. But it makes me wonder if other older or obscure platforms will run into this.

-Peff