RE: [RFC] Avoid invalid parameter warnings in C runtime function for mingw builtr GDB

["Pierre Muller" <pierre.muller@ics-cnrs.unistra.fr>]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Eli Zaretskii
> Envoyé : vendredi 9 août 2013 16:10
> À : Pierre Muller
> Cc : gdb-patches@sourceware.org
> Objet : Re: [RFC] Avoid invalid parameter warnings in C runtime function
for
> mingw builtr GDB
> 
> > From: "Pierre Muller" <pierre.muller@ics-cnrs.unistra.fr>
> > Date: Thu, 8 Aug 2013 10:51:37 +0200
> >
> > Lately I got several warnings with mingw built
> > GDB's:
> >
> > warning: Invalid parameter passed to C runtime function.
> >
> > Use the new "set stop-on-debug-string-event on"
> > command submitted  previously in
> > http://sourceware.org/ml/gdb-patches/2013-08/msg00236.html
> >
> > I was able to trace this down to
> > the fopen call with mode that to "re".
> > As stated in the source, the "e" mode is a glibc extension
> > about close on exec which generates this warning.
> >
> >   The patch below excludes this code if __MINGW32__ is defined,
> > but maybe it should be excluded if O_CLOEXEC is zero?
> 
> Actually, if we want this to be portable, we should have a function to
> make a file handle be non-inheritable, instead of relying on glibc
> extensions.  There _is_ a way to make a file handle non-inheritable on
> Windows.
> 
> > diff -u -p -r1.7 filestuff.c
> > --- src/gdb/common/filestuff.c	26 Jun 2013 08:01:55 -0000	1.7
> > +++ src/gdb/common/filestuff.c	7 Aug 2013 12:35:55 -0000
> > @@ -311,6 +311,7 @@ FILE *
> >  gdb_fopen_cloexec (const char *filename, const char *opentype)
> >  {
> >    FILE *result = NULL;
> > +#ifndef __MINGW32__
> >    static int fopen_e_ever_failed;
> >
> >    if (!fopen_e_ever_failed)
> > @@ -320,17 +321,21 @@ gdb_fopen_cloexec (const char *filename,
> >        copy = alloca (strlen (opentype) + 2);
> >        strcpy (copy, opentype);
> >        /* This is a glibc extension but we try it unconditionally on
> > -	 this path.  */
> > +	 this path, except when using Windows OS msvcrt dll,
> > +	 in order to avoid a output debug string event.  */
> >        strcat (copy, "e");
> >        result = fopen (filename, copy);
> >      }
> > +#endif
> >
> >    if (result == NULL)
> >      {
> >        /* Fallback.  */
> >        result = fopen (filename, opentype);
> > +#ifndef __MINGW32__
> >        if (result != NULL)
> >  	fopen_e_ever_failed = 1;
> > +#endif
> >      }
> >
> >    if (result != NULL)
> 
> Wouldn't it be better to instead initialize fopen_e_ever_failed to 1
> on MinGW?  Then the rest of the code will "just work", no?

  Once again, my insufficient C knowledge is to blame...
Would that be correct?

#if O_CLOEXEC
  static int fopen_e_ever_failed = 0;
#else
  static int fopen_e_ever_failed = 1;
#endif

  But then the variable name is not appropriate anymore...
In fact, when I tried to read the code around that function, I
discovered that there is already another variable called
trust_o_cloexec

  It seems to me that both are testing the same glibc extension,
trust_o_cloexec as it numeric form O_CLOEXEC
and fopen_e_ever_failed as its string mode equivalent "e".
  Is it really possible that trust_o_cloexec and fopen_e_ever_failed
do not match?

  Should a unique variable be enough?

 Concerning my original RFC, should I resubmit it with the above suggestion?

Pierre Muller