Re: [PATCH] Don't assume __secure_getenv is available

[Thomas Schwinge <thomas at codesourcery dot com>]

Hi!

On Fri, 12 May 2017 10:26:59 +0300, Janne Blomqvist <blomqvist.janne@gmail.com> wrote:
> On Fri, May 12, 2017 at 10:23 AM, Thomas Schwinge
> <thomas@codesourcery.com> wrote:
> > On Thu, 27 Apr 2017 21:50:51 +0300, Janne Blomqvist <blomqvist.janne@gmail.com> wrote:
> >> [...], retain the support for __secure_getenv but call it only via a
> >> weak reference.
> >>
> >> Regtested on x86_64-pc-linux-gnu, Ok for trunk, 7.x when it reopens,
> >> 6, 5?
> >
> > Hmm, how has this been tested?  Because:
> >
> >> --- a/libgfortran/runtime/environ.c
> >> +++ b/libgfortran/runtime/environ.c
> >
> >>  #ifdef FALLBACK_SECURE_GETENV
> >> +
> >> +#if SUPPORTS_WEAKREF && defined(HAVE___SECURE_GETENV)
> >> +static char* weak_secure_getenv (const char*)
> >> +  __attribute__((__weakref__("__secure_gettime")));
> >> +#endif
> >
> > "gettime" vs. "getenv"?  ;-)
> 
> Oops. I'm not at my gcc development box now, please consider a patch
> to fix this pre-approved.
> 
> As for testing, I regtested, but my gcc development machine has glibc
> 2.23 which has secure_getenv so it doesn't exercise the fallback
> path..

Then, that clearly isn't an appropriate testing methodology?  What I do
in such cases is manually induce the expected environment (for example,
here I'd probably try hacking out "HAVE_SECURE_GETENV" and
"HAVE___SECURE_GETENV" from the generated "config.h" or even
"libgfortran/configure"), and then manually run something that is
expected to behave differently in an environment relevant to
"secure_getenv" -- I doubt that such a thing is being (or, can really be)
included in the GCC testsuite?

Still untested -- ;-) -- but including another typo fix, committed to
trunk in r247952:

commit bc9457364b4b9a847c91e35a0aa5fc3b73df53a0
Author: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Fri May 12 07:56:41 2017 +0000

    Typo fixes for "Don't assume __secure_getenv is available"
    
            libgfortran/
            * runtime/environ.c (weak_secure_getenv): Fix "__secure_gettime"
            vs. "__secure_getenv" typo.
            (secure_getenv): Fix "HAVE__SECURE_GETENV"
            vs. "HAVE___SECURE_GETENV" typo.
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@247952 138bc75d-0d04-0410-961f-82ee72b054a4
---
 libgfortran/ChangeLog         | 7 +++++++
 libgfortran/runtime/environ.c | 4 ++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git libgfortran/ChangeLog libgfortran/ChangeLog
index 337daaf..6b7da0a 100644
--- libgfortran/ChangeLog
+++ libgfortran/ChangeLog
@@ -1,3 +1,10 @@
+2017-05-12  Thomas Schwinge  <thomas@codesourcery.com>
+
+	* runtime/environ.c (weak_secure_getenv): Fix "__secure_gettime"
+	vs. "__secure_getenv" typo.
+	(secure_getenv): Fix "HAVE__SECURE_GETENV"
+	vs. "HAVE___SECURE_GETENV" typo.
+
 2017-05-11  Janne Blomqvist  <jb@gcc.gnu.org>
 
 	* libgfortran.h: HAVE_SECURE_GETENV: Don't check
diff --git libgfortran/runtime/environ.c libgfortran/runtime/environ.c
index 969dcdf..f0a593e 100644
--- libgfortran/runtime/environ.c
+++ libgfortran/runtime/environ.c
@@ -40,13 +40,13 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 
 #if SUPPORTS_WEAKREF && defined(HAVE___SECURE_GETENV)
 static char* weak_secure_getenv (const char*)
-  __attribute__((__weakref__("__secure_gettime")));
+  __attribute__((__weakref__("__secure_getenv")));
 #endif
 
 char *
 secure_getenv (const char *name)
 {
-#if SUPPORTS_WEAKREF && defined(HAVE__SECURE_GETENV)
+#if SUPPORTS_WEAKREF && defined(HAVE___SECURE_GETENV)
   if (weak_secure_getenv)
     return weak_secure_getenv (name);
 #endif


Grüße
 Thomas