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

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

[Eli Zaretskii]

> 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?

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

[Pierre Muller]

> -----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
  

 

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

[Tom Tromey]

>>>>> "Pierre" == Pierre Muller <pierre.muller@ics-cnrs.unistra.fr> writes:

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

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

O_CLOEXEC is unconditionally defined earlier.
But you can just write:

  /* If we provide a fake O_CLOEXEC, then don't ever try "e".  */
  static int fopen_e_ever_failed = O_CLOEXEC == 0;

Pierre>   But then the variable name is not appropriate anymore...

You can rename it if you want, but I don't really care either way.

Pierre> In fact, when I tried to read the code around that function, I
Pierre> discovered that there is already another variable called
Pierre> trust_o_cloexec

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

I forget exactly why I did this.
Maybe I was just being overly cautious.

Pierre>   Should a unique variable be enough?

Maybe.

Tom

[RFA-v2] Avoid invalid parameter warnings in C runtime function for mingw built GDB

[Pierre Muller]

  I followed Eli's advice to go with Tom's suggestion.

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Tom Tromey
> Envoyé : mardi 13 août 2013 17:38
> À : Pierre Muller
> Cc : 'Eli Zaretskii'; gdb-patches@sourceware.org
> Objet : Re: [RFC] Avoid invalid parameter warnings in C runtime function
for
> mingw builtr GDB
> 
> >>>>> "Pierre" == Pierre Muller <pierre.muller@ics-cnrs.unistra.fr>
writes:
> 
> Pierre>   Once again, my insufficient C knowledge is to blame...
> Pierre> Would that be correct?
> 
> Pierre> #if O_CLOEXEC
> Pierre>   static int fopen_e_ever_failed = 0;
> Pierre> #else
> Pierre>   static int fopen_e_ever_failed = 1;
> Pierre> #endif
> 
> O_CLOEXEC is unconditionally defined earlier.
> But you can just write:
> 
>   /* If we provide a fake O_CLOEXEC, then don't ever try "e".  */
>   static int fopen_e_ever_failed = O_CLOEXEC == 0;

Is this OK to commit?
Maybe some comments on the ChangeLog entry?


Pierre Muller


2013-08-14  Pierre Muller  <muller@sourceware.org>
	    Tom Tromey  <tromey@redhat.com>

	* common/filestuff.c (gdb_fopen_cloexec): Do not try to use "e"
	mode if operating system doesn't know O_CLOEXEC, this allows to
	avoid getting a output debug string warning for mingw hosted
	GDB executables.


Index: src/gdb/common/filestuff.c
===================================================================
RCS file: /cvs/src/src/gdb/common/filestuff.c,v
retrieving revision 1.7
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	14 Aug 2013 09:44:40 -0000
@@ -311,7 +311,9 @@ FILE *
 gdb_fopen_cloexec (const char *filename, const char *opentype)
 {
   FILE *result = NULL;
-  static int fopen_e_ever_failed;
+  /* If O_CLOEXEC is zero, the operating system doesn't
+     know about close on exec mode "e", so don't even try to use it.  */
+  static int fopen_e_ever_failed = O_CLOEXEC == 0;
 
   if (!fopen_e_ever_failed)
     {

Re: [RFA-v2] Avoid invalid parameter warnings in C runtime function for mingw built GDB

[Pedro Alves]

On 08/14/2013 12:38 PM, Pierre Muller wrote:

> Is this OK to commit?
> Maybe some comments on the ChangeLog entry?

> 	* common/filestuff.c (gdb_fopen_cloexec): Do not try to use "e"
> 	mode if operating system doesn't know O_CLOEXEC, this allows to
> 	avoid getting a output debug string warning for mingw hosted
> 	GDB executables.

This comment should really be in the sources instead.  That here you'd
have:

	* common/filestuff.c (gdb_fopen_cloexec): Do not try to use "e"
	mode if operating system doesn't know O_CLOEXEC.

and in the source, where you have:

> +  /* If O_CLOEXEC is zero, the operating system doesn't
> +     know about close on exec mode "e", so don't even try to use it.  */
> +  static int fopen_e_ever_failed = O_CLOEXEC == 0;

I suggest:

  /* Probe for "e" support once.  But, if we can tell the operating
     system doesn't know about close on exec mode "e" without probing,
     skip it.  E.g., the Windows runtime issues an "Invalid parameter
     passed to C runtime function" OutputDebugString warning for
     unknown modes.  Assume that if O_CLOEXEC is zero, then "e" isn't
     supported.  */
  static int fopen_e_ever_failed;

-- 
Pedro Alves

[RFA-v3] Avoid invalid parameter warnings in C runtime function for mingw built GDB

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pedro Alves
> Envoyé : mercredi 14 août 2013 14:02
> À : Pierre Muller
> Cc : 'Tom Tromey'; 'Eli Zaretskii'; gdb-patches@sourceware.org
> Objet : Re: [RFA-v2] Avoid invalid parameter warnings in C runtime
function
> for mingw built GDB
> 
> On 08/14/2013 12:38 PM, Pierre Muller wrote:
> 
> > Is this OK to commit?
> > Maybe some comments on the ChangeLog entry?
> 
> > 	* common/filestuff.c (gdb_fopen_cloexec): Do not try to use "e"
> > 	mode if operating system doesn't know O_CLOEXEC, this allows to
> > 	avoid getting a output debug string warning for mingw hosted
> > 	GDB executables.
> 
> This comment should really be in the sources instead.  That here you'd
> have:
> 
> 	* common/filestuff.c (gdb_fopen_cloexec): Do not try to use "e"
> 	mode if operating system doesn't know O_CLOEXEC.

  I knew that the ChangeLog entry was not what is usually expected...
But your version seems perfect!
 
> and in the source, where you have:
> 
> > +  /* If O_CLOEXEC is zero, the operating system doesn't
> > +     know about close on exec mode "e", so don't even try to use it.
*/
> > +  static int fopen_e_ever_failed = O_CLOEXEC == 0;
> 
> I suggest:
> 
>   /* Probe for "e" support once.  But, if we can tell the operating
>      system doesn't know about close on exec mode "e" without probing,
>      skip it.  E.g., the Windows runtime issues an "Invalid parameter
>      passed to C runtime function" OutputDebugString warning for
>      unknown modes.  Assume that if O_CLOEXEC is zero, then "e" isn't
>      supported.  */
>   static int fopen_e_ever_failed;
  Your comment is much better indeed, but let's still keep the 
static int fopen_e_ever_failed = O_CLOEXEC == 0;
line, which is the only real change to the code...

> --
> Pedro Alves

Thanks Pedro!

Is this third version OK?


2013-08-14  Pierre Muller  <muller@sourceware.org>
	    Tom Tromey  <tromey@redhat.com>
	    Pedro Alves  <palves@redhat.com>

	* common/filestuff.c (gdb_fopen_cloexec): Do not try to use "e"
	mode if operating system doesn't know O_CLOEXEC.


Index: common/filestuff.c
===================================================================
RCS file: /cvs/src/src/gdb/common/filestuff.c,v
retrieving revision 1.7
diff -u -p -r1.7 filestuff.c
--- common/filestuff.c	26 Jun 2013 08:01:55 -0000	1.7
+++ common/filestuff.c	14 Aug 2013 12:07:02 -0000
@@ -311,7 +311,13 @@ FILE *
 gdb_fopen_cloexec (const char *filename, const char *opentype)
 {
   FILE *result = NULL;
-  static int fopen_e_ever_failed;
+  /* Probe for "e" support once.  But, if we can tell the operating
+     system doesn't know about close on exec mode "e" without probing,
+     skip it.  E.g., the Windows runtime issues an "Invalid parameter
+     passed to C runtime function" OutputDebugString warning for
+     unknown modes.  Assume that if O_CLOEXEC is zero, then "e" isn't
+     supported.  */
+  static int fopen_e_ever_failed = O_CLOEXEC == 0;
 
   if (!fopen_e_ever_failed)
     {

RE: [RFA-v3] Avoid invalid parameter warnings in C runtime function for mingw built GDB

[Pierre Muller]

Ping?

Nobody reacted to this third version...

Pierre Muller


> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pierre Muller
> Envoyé : mercredi 14 août 2013 14:13
> À : 'Pedro Alves'
> Cc : 'Tom Tromey'; 'Eli Zaretskii'; gdb-patches@sourceware.org
> Objet : [RFA-v3] Avoid invalid parameter warnings in C runtime function
for
> mingw built GDB
> 
> 
> 
> > -----Message d'origine-----
> > De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> > owner@sourceware.org] De la part de Pedro Alves
> > Envoyé : mercredi 14 août 2013 14:02
> > À : Pierre Muller
> > Cc : 'Tom Tromey'; 'Eli Zaretskii'; gdb-patches@sourceware.org
> > Objet : Re: [RFA-v2] Avoid invalid parameter warnings in C runtime
> function
> > for mingw built GDB
> >
> > On 08/14/2013 12:38 PM, Pierre Muller wrote:
> >
> > > Is this OK to commit?
> > > Maybe some comments on the ChangeLog entry?
> >
> > > 	* common/filestuff.c (gdb_fopen_cloexec): Do not try to use "e"
> > > 	mode if operating system doesn't know O_CLOEXEC, this allows to
> > > 	avoid getting a output debug string warning for mingw hosted
> > > 	GDB executables.
> >
> > This comment should really be in the sources instead.  That here you'd
> > have:
> >
> > 	* common/filestuff.c (gdb_fopen_cloexec): Do not try to use "e"
> > 	mode if operating system doesn't know O_CLOEXEC.
> 
>   I knew that the ChangeLog entry was not what is usually expected...
> But your version seems perfect!
> 
> > and in the source, where you have:
> >
> > > +  /* If O_CLOEXEC is zero, the operating system doesn't
> > > +     know about close on exec mode "e", so don't even try to use it.
> */
> > > +  static int fopen_e_ever_failed = O_CLOEXEC == 0;
> >
> > I suggest:
> >
> >   /* Probe for "e" support once.  But, if we can tell the operating
> >      system doesn't know about close on exec mode "e" without probing,
> >      skip it.  E.g., the Windows runtime issues an "Invalid parameter
> >      passed to C runtime function" OutputDebugString warning for
> >      unknown modes.  Assume that if O_CLOEXEC is zero, then "e" isn't
> >      supported.  */
> >   static int fopen_e_ever_failed;
>   Your comment is much better indeed, but let's still keep the
> static int fopen_e_ever_failed = O_CLOEXEC == 0;
> line, which is the only real change to the code...
> 
> > --
> > Pedro Alves
> 
> Thanks Pedro!
> 
> Is this third version OK?
> 
> 
> 2013-08-14  Pierre Muller  <muller@sourceware.org>
> 	    Tom Tromey  <tromey@redhat.com>
> 	    Pedro Alves  <palves@redhat.com>
> 
> 	* common/filestuff.c (gdb_fopen_cloexec): Do not try to use "e"
> 	mode if operating system doesn't know O_CLOEXEC.
> 
> 
> Index: common/filestuff.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/common/filestuff.c,v
> retrieving revision 1.7
> diff -u -p -r1.7 filestuff.c
> --- common/filestuff.c	26 Jun 2013 08:01:55 -0000	1.7
> +++ common/filestuff.c	14 Aug 2013 12:07:02 -0000
> @@ -311,7 +311,13 @@ FILE *
>  gdb_fopen_cloexec (const char *filename, const char *opentype)
>  {
>    FILE *result = NULL;
> -  static int fopen_e_ever_failed;
> +  /* Probe for "e" support once.  But, if we can tell the operating
> +     system doesn't know about close on exec mode "e" without probing,
> +     skip it.  E.g., the Windows runtime issues an "Invalid parameter
> +     passed to C runtime function" OutputDebugString warning for
> +     unknown modes.  Assume that if O_CLOEXEC is zero, then "e" isn't
> +     supported.  */
> +  static int fopen_e_ever_failed = O_CLOEXEC == 0;
> 
>    if (!fopen_e_ever_failed)
>      {

Re: [RFA-v3] Avoid invalid parameter warnings in C runtime function for mingw built GDB

[Doug Evans]

Hi.
Patch is ok with me.

On Fri, Sep 13, 2013 at 3:35 PM, Pierre Muller
<pierre.muller@ics-cnrs.unistra.fr> wrote:
> Ping?
>
> Nobody reacted to this third version...
>
> Pierre Muller
>
>
>> -----Message d'origine-----
>> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
>> owner@sourceware.org] De la part de Pierre Muller
>> Envoyé : mercredi 14 août 2013 14:13
>> À : 'Pedro Alves'
>> Cc : 'Tom Tromey'; 'Eli Zaretskii'; gdb-patches@sourceware.org
>> Objet : [RFA-v3] Avoid invalid parameter warnings in C runtime function
> for
>> mingw built GDB
>>
>>
>>
>> > -----Message d'origine-----
>> > De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
>> > owner@sourceware.org] De la part de Pedro Alves
>> > Envoyé : mercredi 14 août 2013 14:02
>> > À : Pierre Muller
>> > Cc : 'Tom Tromey'; 'Eli Zaretskii'; gdb-patches@sourceware.org
>> > Objet : Re: [RFA-v2] Avoid invalid parameter warnings in C runtime
>> function
>> > for mingw built GDB
>> >
>> > On 08/14/2013 12:38 PM, Pierre Muller wrote:
>> >
>> > > Is this OK to commit?
>> > > Maybe some comments on the ChangeLog entry?
>> >
>> > >   * common/filestuff.c (gdb_fopen_cloexec): Do not try to use "e"
>> > >   mode if operating system doesn't know O_CLOEXEC, this allows to
>> > >   avoid getting a output debug string warning for mingw hosted
>> > >   GDB executables.
>> >
>> > This comment should really be in the sources instead.  That here you'd
>> > have:
>> >
>> >     * common/filestuff.c (gdb_fopen_cloexec): Do not try to use "e"
>> >     mode if operating system doesn't know O_CLOEXEC.
>>
>>   I knew that the ChangeLog entry was not what is usually expected...
>> But your version seems perfect!
>>
>> > and in the source, where you have:
>> >
>> > > +  /* If O_CLOEXEC is zero, the operating system doesn't
>> > > +     know about close on exec mode "e", so don't even try to use it.
>> */
>> > > +  static int fopen_e_ever_failed = O_CLOEXEC == 0;
>> >
>> > I suggest:
>> >
>> >   /* Probe for "e" support once.  But, if we can tell the operating
>> >      system doesn't know about close on exec mode "e" without probing,
>> >      skip it.  E.g., the Windows runtime issues an "Invalid parameter
>> >      passed to C runtime function" OutputDebugString warning for
>> >      unknown modes.  Assume that if O_CLOEXEC is zero, then "e" isn't
>> >      supported.  */
>> >   static int fopen_e_ever_failed;
>>   Your comment is much better indeed, but let's still keep the
>> static int fopen_e_ever_failed = O_CLOEXEC == 0;
>> line, which is the only real change to the code...
>>
>> > --
>> > Pedro Alves
>>
>> Thanks Pedro!
>>
>> Is this third version OK?
>>
>>
>> 2013-08-14  Pierre Muller  <muller@sourceware.org>
>>           Tom Tromey  <tromey@redhat.com>
>>           Pedro Alves  <palves@redhat.com>
>>
>>       * common/filestuff.c (gdb_fopen_cloexec): Do not try to use "e"
>>       mode if operating system doesn't know O_CLOEXEC.
>>
>>
>> Index: common/filestuff.c
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/common/filestuff.c,v
>> retrieving revision 1.7
>> diff -u -p -r1.7 filestuff.c
>> --- common/filestuff.c        26 Jun 2013 08:01:55 -0000      1.7
>> +++ common/filestuff.c        14 Aug 2013 12:07:02 -0000
>> @@ -311,7 +311,13 @@ FILE *
>>  gdb_fopen_cloexec (const char *filename, const char *opentype)
>>  {
>>    FILE *result = NULL;
>> -  static int fopen_e_ever_failed;
>> +  /* Probe for "e" support once.  But, if we can tell the operating
>> +     system doesn't know about close on exec mode "e" without probing,
>> +     skip it.  E.g., the Windows runtime issues an "Invalid parameter
>> +     passed to C runtime function" OutputDebugString warning for
>> +     unknown modes.  Assume that if O_CLOEXEC is zero, then "e" isn't
>> +     supported.  */
>> +  static int fopen_e_ever_failed = O_CLOEXEC == 0;
>>
>>    if (!fopen_e_ever_failed)
>>      {

RE: [RFA-v3] Avoid invalid parameter warnings in C runtime function for mingw built GDB

[Pierre Muller]

Thank you for the approval.


For the record, below is what I committed.

This patch suppresses this warning:
warning: Invalid parameter passed to C runtime function.
that appeared for *-*-mingw* hosts
when debugging gdb with itself.

Pierre Muller


2013-09-14  Pierre Muller  <muller@sourceware.org>
            Tom Tromey  <tromey@redhat.com>
            Pedro Alves  <palves@redhat.com>

        * common/filestuff.c (gdb_fopen_cloexec): Do not try to use "e"
        mode if operating system doesn't know O_CLOEXEC.


Index: src/gdb/common/filestuff.c
===================================================================
RCS file: /cvs/src/src/gdb/common/filestuff.c,v
retrieving revision 1.7
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  14 Aug 2013 12:07:02 -0000
@@ -311,7 +311,13 @@ FILE *
 gdb_fopen_cloexec (const char *filename, const char *opentype)
 {
   FILE *result = NULL;
-  static int fopen_e_ever_failed;
+  /* Probe for "e" support once.  But, if we can tell the operating
+     system doesn't know about close on exec mode "e" without probing,
+     skip it.  E.g., the Windows runtime issues an "Invalid parameter
+     passed to C runtime function" OutputDebugString warning for
+     unknown modes.  Assume that if O_CLOEXEC is zero, then "e" isn't
+     supported.  */
+  static int fopen_e_ever_failed = O_CLOEXEC == 0;

   if (!fopen_e_ever_failed)
     {




> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Doug Evans
> Envoyé : samedi 14 septembre 2013 01:08
> À : Pierre Muller
> Cc : gdb-patches; Tom Tromey; Eli Zaretskii; Pedro Alves
> Objet : Re: [RFA-v3] Avoid invalid parameter warnings in C runtime
function
> for mingw built GDB
> 
> Hi.
> Patch is ok with me.
> 
> On Fri, Sep 13, 2013 at 3:35 PM, Pierre Muller
> <pierre.muller@ics-cnrs.unistra.fr> wrote:
> > Ping?
> >
> > Nobody reacted to this third version...
> >
> > Pierre Muller
> >
> >
> >> -----Message d'origine-----
> >> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> >> owner@sourceware.org] De la part de Pierre Muller
> >> Envoyé : mercredi 14 août 2013 14:13
> >> À : 'Pedro Alves'
> >> Cc : 'Tom Tromey'; 'Eli Zaretskii'; gdb-patches@sourceware.org
> >> Objet : [RFA-v3] Avoid invalid parameter warnings in C runtime function
> > for
> >> mingw built GDB
> >>
> >>
> >>
> >> > -----Message d'origine-----
> >> > De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> >> > owner@sourceware.org] De la part de Pedro Alves
> >> > Envoyé : mercredi 14 août 2013 14:02
> >> > À : Pierre Muller
> >> > Cc : 'Tom Tromey'; 'Eli Zaretskii'; gdb-patches@sourceware.org
> >> > Objet : Re: [RFA-v2] Avoid invalid parameter warnings in C runtime
> >> function
> >> > for mingw built GDB
> >> >
> >> > On 08/14/2013 12:38 PM, Pierre Muller wrote:
> >> >
> >> > > Is this OK to commit?
> >> > > Maybe some comments on the ChangeLog entry?
> >> >
> >> > >   * common/filestuff.c (gdb_fopen_cloexec): Do not try to use "e"
> >> > >   mode if operating system doesn't know O_CLOEXEC, this allows to
> >> > >   avoid getting a output debug string warning for mingw hosted
> >> > >   GDB executables.
> >> >
> >> > This comment should really be in the sources instead.  That here
you'd
> >> > have:
> >> >
> >> >     * common/filestuff.c (gdb_fopen_cloexec): Do not try to use "e"
> >> >     mode if operating system doesn't know O_CLOEXEC.
> >>
> >>   I knew that the ChangeLog entry was not what is usually expected...
> >> But your version seems perfect!
> >>
> >> > and in the source, where you have:
> >> >
> >> > > +  /* If O_CLOEXEC is zero, the operating system doesn't
> >> > > +     know about close on exec mode "e", so don't even try to use
it.
> >> */
> >> > > +  static int fopen_e_ever_failed = O_CLOEXEC == 0;
> >> >
> >> > I suggest:
> >> >
> >> >   /* Probe for "e" support once.  But, if we can tell the operating
> >> >      system doesn't know about close on exec mode "e" without
probing,
> >> >      skip it.  E.g., the Windows runtime issues an "Invalid parameter
> >> >      passed to C runtime function" OutputDebugString warning for
> >> >      unknown modes.  Assume that if O_CLOEXEC is zero, then "e" isn't
> >> >      supported.  */
> >> >   static int fopen_e_ever_failed;
> >>   Your comment is much better indeed, but let's still keep the
> >> static int fopen_e_ever_failed = O_CLOEXEC == 0;
> >> line, which is the only real change to the code...
> >>
> >> > --
> >> > Pedro Alves
> >>
> >> Thanks Pedro!
> >>
> >> Is this third version OK?
> >>
> >>
> >> 2013-08-14  Pierre Muller  <muller@sourceware.org>
> >>           Tom Tromey  <tromey@redhat.com>
> >>           Pedro Alves  <palves@redhat.com>
> >>
> >>       * common/filestuff.c (gdb_fopen_cloexec): Do not try to use "e"
> >>       mode if operating system doesn't know O_CLOEXEC.
> >>
> >>
> >> Index: common/filestuff.c
> >> ===================================================================
> >> RCS file: /cvs/src/src/gdb/common/filestuff.c,v
> >> retrieving revision 1.7
> >> diff -u -p -r1.7 filestuff.c
> >> --- common/filestuff.c        26 Jun 2013 08:01:55 -0000      1.7
> >> +++ common/filestuff.c        14 Aug 2013 12:07:02 -0000
> >> @@ -311,7 +311,13 @@ FILE *
> >>  gdb_fopen_cloexec (const char *filename, const char *opentype)
> >>  {
> >>    FILE *result = NULL;
> >> -  static int fopen_e_ever_failed;
> >> +  /* Probe for "e" support once.  But, if we can tell the operating
> >> +     system doesn't know about close on exec mode "e" without probing,
> >> +     skip it.  E.g., the Windows runtime issues an "Invalid parameter
> >> +     passed to C runtime function" OutputDebugString warning for
> >> +     unknown modes.  Assume that if O_CLOEXEC is zero, then "e" isn't
> >> +     supported.  */
> >> +  static int fopen_e_ever_failed = O_CLOEXEC == 0;
> >>
> >>    if (!fopen_e_ever_failed)
> >>      {

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

[Eli Zaretskii]

> From: "Pierre Muller" <pierre.muller@ics-cnrs.unistra.fr>
> Cc: <gdb-patches@sourceware.org>
> Date: Tue, 13 Aug 2013 11:13:49 +0200
> 
> Would that be correct?
> 
> #if O_CLOEXEC
>   static int fopen_e_ever_failed = 0;
> #else
>   static int fopen_e_ever_failed = 1;
> #endif

Yes, but Tom's suggestion is more elegant.

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

[Tom Tromey]

>>>>> "Pierre" == Pierre Muller <pierre.muller@ics-cnrs.unistra.fr> writes:

Pierre>   The patch below excludes this code if __MINGW32__ is defined,
Pierre> but maybe it should be excluded if O_CLOEXEC is zero?

That would be fine by me.

Tom

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

[Pierre Muller]

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?

Comments welcome.


Pierre Muller
GDB pascal language maintainer

2013-08-07  Pierre Muller  <muller@sourceware.org>

	* src/gdb/common/filestuff.c (gdb_fopen_cloexec): Do not
	try to use "e" mode if linking to Windows OS msvcrt DLL,
	to avoid getting a output debug string event.


Index: src/gdb/common/filestuff.c
===================================================================
RCS file: /cvs/src/src/gdb/common/filestuff.c,v
retrieving revision 1.7
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)