Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Charles Wilson <cygwin@cwilson.fastmail.fm>
Cc: gdb-patches@sourceware.org
Subject: Re: RFA: ensure binary objects opened in binary mode
Date: Wed, 22 Feb 2006 18:50:00 -0000	[thread overview]
Message-ID: <uirr7kzsh.fsf@gnu.org> (raw)
In-Reply-To: <43FBF706.9030604@cwilson.fastmail.fm> (message from Charles 	Wilson on Wed, 22 Feb 2006 00:30:46 -0500)

> Date: Wed, 22 Feb 2006 00:30:46 -0500
> From: Charles Wilson <cygwin@cwilson.fastmail.fm>
> CC:  gdb-patches@sourceware.org
> 
> Okay, I've attached two patches that hopefully address all the issues 
> raised in this thread.

Thanks.

> (1) for every file that #includes both defs.h AND <fcntl.h>, remove the 
> <fcntl.h> inclusion.

I'm not sure this is a good idea.  What if tomorrow we remove fcntl.h
from defs.h--do we go through all these files again and add it back?
Why bother? fcntl.h should be idempotent, so including it several
times does no real harm.

I actually quite dislike source files that don't include standard
headers because they are included in defs.h and its ilk.  It makes me
wonder how come foo.c uses something defined in bar.h, but there's no
"#include <bar.h>" anywhere in sight.

> (2) Further, for every file that contains the #ifndef...#define 
> O_BINARY...#endif stanza (all of which #include defs.h), that stanza is 
> removed (3 line change to each of 5 files)

This is what I think we wanted.  We don't want O_BINARY defined
identically in several places.

So I'd prefer if you committed the 1st and the 3rd patch. but not the
second.  However, before you actually do that, let's wait and hear
what others think.

> +/* In case this is not defined in fcntl.h */
> +
> +#ifndef O_BINARY
> +#define O_BINARY 0
> +#endif

I'd change the comment to explain that O_BINARY has a meaning on
non-Posix platforms, while on Posix platforms it should be a no-op.
That is the _real_ reason we define O_BINARY.


  parent reply	other threads:[~2006-02-22 18:36 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-02-17 22:02 Charles Wilson
2006-02-17 23:41 ` Christopher Faylor
2006-02-18 10:45   ` Eli Zaretskii
2006-02-18 11:19     ` Mark Kettenis
2006-02-18 11:47       ` Eli Zaretskii
2006-02-18 11:55         ` Mark Kettenis
2006-02-22  5:52         ` Charles Wilson
2006-02-22 17:06           ` Christopher Faylor
2006-02-22 18:50           ` Eli Zaretskii [this message]
2006-02-22 21:55             ` Daniel Jacobowitz
2006-02-24 10:37               ` Charles Wilson
2006-02-24 11:44                 ` Eli Zaretskii
2006-02-25  7:29                   ` Charles Wilson
2006-03-01 22:11             ` Michael Snyder
2006-02-18 10:50 ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=uirr7kzsh.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=cygwin@cwilson.fastmail.fm \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox