RE: [PATCH 01/12] gdb, testsuite: Rename set_sanitizer_default to append_environment.

["Schimpe, Christina" <christina.schimpe@intel.com>]

> -----Original Message-----
> From: Guinevere Larsen <guinevere@redhat.com>
> Sent: Tuesday, January 28, 2025 2:45 PM
> To: Schimpe, Christina <christina.schimpe@intel.com>; gdb-
> patches@sourceware.org
> Subject: Re: [PATCH 01/12] gdb, testsuite: Rename set_sanitizer_default to
> append_environment.
> 
> On 12/20/24 5:04 PM, Schimpe, Christina wrote:
> > The procedure set_sanitizer_default has been used for the
> > configuration of ASAN specific environment variables.  However, it is
> > actually a generic function.  Rename it to append_environment so that
> > its purpose is more clear.
> 
> I think this is a good change, but in that case we should also rename
> set_sanitizer_1 and set_sanitizer. I think the conversion should be:
> 
> set_sanitizer -> append_environment
> set_sanitizer_1 -> append_environment_1
> set_sanitizer_default -> append_environment_default
> 
> Also, I can see that patch 12 (and maybe others) use the append_environment
> call. If the user had already set GLIBC_TUNABLES, the test wouldn't update the
> value if I understand the TCL code correctly.
> Is that the expected outcome? If not, I would suggest the alternative of renaming
> set_sanitizer_1 to append_environment, so that set_sanitizer* can continue to
> work as is, and you can manually set the environment variables in a more obvious
> way.

Thank you for the feedback. Yes, if the environment variable is already configured we
don't configure it by calling set_sanitizer_default/append_environment_default, in 
contrast to set_sanitizer_1.  I think set_sanitizer_1 was introduced later, that's why I
probably missed it.

So I totally agree, we should rename all functions and your suggestion

> set_sanitizer -> append_environment
> set_sanitizer_1 -> append_environment_1
> set_sanitizer_default -> append_environment_default

makes sense to me. 

This patch is actually independent of this series and wonder if I could post it separately, as I expect to merge it sooner then.
I am adding Tom to this conversation who introduced these procedures. Does the renaming as suggested above make sense to you?

Thanks!
Christina
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928