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

[Doug Evans <dje@google.com>]

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