Re: [PATCH] test-lib.sh: Use GLIBC_TUNABLES instead of MALLOC_CHECK_ on glibc >= 2.34
- From
brian m. carlson <sandals@crustytoothpaste.net>
- Date
- Mar 1, 2022, 01:25 UTC
- Message-ID
- <Yh12FNTGuJkRlqBE@camp.crustytoothpaste.net>
- In-Reply-To
- <xmqq7d9e249n.fsf@gitster.g>
On 2022-02-28 at 19:13:40, Junio C Hamano wrote:
Show 8 quoted lines
> Elia Pinto <gitter.spiros@gmail.com> writes: > > > In glibc >= 2.34 MALLOC_CHECK_ and MALLOC_PERTURB_ environment > > variables have been replaced by GLIBC_TUNABLES. Also the new > > Does it hurt to have these older environment variables? If not, > we would prefer to see redundant but less deeply indented code, > I would imagine.
Setting both sets of environment variables is probably just fine and the simplest solution, I'd imagine.
Show 12 quoted lines
> > + if type -p getconf >/dev/null 2>&1; then
> > + _GLIBC_VERSION="$(getconf GNU_LIBC_VERSION 2>/dev/null | awk '{ print $2 }')"
> > + if [ -n "$_GLIBC_VERSION" -a $(expr "$_GLIBC_VERSION" \>= "2.34") ]; then
> > + _HAVE_GLIBC_234="yes"
> > + fi
> > + fi
>
> Style. We prefer "test ..." over "[ ... ]" and more importantly we
> don't use "test X -a Y".
>
> Do we absolutely need "test -p getconf" with an extra indentation?
> I suspect we don't.getconf is specified by POSIX, but that doesn't mean it's in the default install or in PATH on all systems. However, we should write "command -v getconf" instead if we need to check, since that's the POSIX way to write it, and "type" is not guaranteed to be present on all systems.
It looks like the code might silently not match if getconf isn't present, and if so then we can avoid the check, but we should of course put a comment noting that behavior to help future readers.
-- brian m. carlson (he/him or they/them) Toronto, Ontario, CA