RFA: ensure binary objects opened in binary mode

Charles Wilson cygwin@cwilson.fastmail.fm
Wed Feb 22 05:52:00 GMT 2006


Eli Zaretskii wrote:
>> Date: Sat, 18 Feb 2006 12:18:58 +0100 (CET)
>> From: Mark Kettenis <mark.kettenis@xs4all.nl>
>> CC: gdb-patches@sourceware.org, cygwin@cwilson.fastmail.fm
>>
>>> Agreed.  I think defs.h is a good place, do you agree?
>>>
>> Problem is that right now defs.h doesn't include <fcntl.h> (where I
>> expect O_BINARY to be defined on systems that have it).  So we should
>> move that into fcntl.h as well
> 
> You mean, include <fcntl.h> from defs.h, right?  If so, I agree.

Okay, I've attached two patches that hopefully address all the issues 
raised in this thread.  Both are against gdb-6.4.50-20060131 but should 
apply cleanly to HEAD.  The first one is all that is *strictly* 
necessary to correct the bug:

  defs.h  |    8 ++++++++
  solib.c |   13 ++++++-------
  2 files changed, 14 insertions(+), 7 deletions(-)

ChangeLog (patch 1):

2006-02-21  Charles Wilson  <cygwin@...>

	* gdb/defs.h: unconditionally include <fcntl.h>, and
	ensure that O_BINARY is defined.
	* gdb/solib.c(solib_open): ensure solib files are opened in
	binary mode. Remove <fcntl.h>.

I believe this patch still counts as "trivial" by FSF guidelines: 4 
non-blank/non-comment line additions to defs.h and 7 non-blank line 
changes to solib.c (and six of those are identical).  However...

+++++++++++++++++++++++++++++++++++++

The second patch does the following (filenames below are relative to the 
src/gdb/ directory):

===============
(1) for every file that #includes both defs.h AND <fcntl.h>, remove the 
<fcntl.h> inclusion.  This is a one-line change to each of the following 
48 files:

auxv.c                          inflow.c            rs6000-nat.c
bsd-kvm.c                       inftarg.c           ser-pipe.c
cli/cli-cmds.c                  linux-nat.c         ser-unix.c
core-regset.c                   m68klinux-nat.c     sol-thread.c
corefile.c                      nto-procfs.c        solib-aix5.c
corelow.c                       objfiles.c          solib-sunos.c
dbxread.c                       ocd.c               source.c
dwarf2read.c                    ppc-bdm.c           symfile.c
dwarfread.c                     ppc-linux-nat.c     symtab.c
exec.c                          proc-api.c          tui/tui-hooks.c
gdbtk/generic/gdbtk-cmds.c      procfs.c            tui/tui-io.c
gdbtk/generic/gdbtk-hooks.c     remote-fileio.c     tui/tui.c
gdbtk/generic/gdbtk.c           remote-rdp.c        uw-thread.c
go32-nat.c                      remote-sds.c        win32-nat.c
hpux-thread.c                   remote-sim.c        wince.c
i386v-nat.c                     remote.c            xcoffread.c

These changes were automatically performed using the following silly 
script, executed from the src/gdb directory:

#!/bin/bash
## execute from src/gdb directory
##
(for fn in `find . -type f | xargs grep -l fcntl.h` ; do
   grep -l defs.h $fn;
done) |\
   sed -e '/ChangeLog/d' -e '/Makefile/d' -e '/configure/d' |\
   (while read LN ; do
      echo "Fixing ${LN}"
      cat "${LN}" | sed -e '/#include <fcntl.h>.*/d' > "${LN}.new"
      if [ -e "${LN}.new" ] ; then
        mv "${LN}" "${LN}.old"
        mv "${LN}.new" "${LN}"
      fi
   done)
find . -name "*.old" | xargs rm -f

===============
(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)

corelow.c    exec.c    remote-rdp.c    source.c    symfile.c

These changes, too, were done via the following silly script executed 
from the src/gdb directory:

#!/bin/bash
## execute from src/gdb directory.
##
## note: the line which begins with /^#ifndef should have no
## linebreaks until the $/d    Plus, in that line there is no
## whitespace between ]* and define[ 	]
## Finally, each [ 	] in that line contains a space and a tab.
for fn in corelow.c exec.c remote-rdp.c source.c symfile.c ; do
   echo "Fixing ${fn}"
   cat "${fn}" | sed -e ': more
      $!N
      s/\n/&/2;
      t enough
      $!b more
      : enough
      /^#ifndef[ 	]\+O_BINARY[ 	]*[^\n]*\n#[ 	]*
define[ 	]\+O_BINARY[ 	]*[^\n]*\n#endif[ 	]*$/d
      P;D' > "${fn}.new"
   if [ -e "${fn}.new" ] ; then
     mv "${fn}" "${fn}.old"
     mv "${fn}.new" "${fn}"
   fi
done
find . -name "*.old" | xargs rm -f

===============

My point: while the concept in the second patch is extremely simple (so 
simple that a pair of automated scripts can create it), it still ends up 
modifying 48 different files, deleting 1 (or 4) lines from each.  I'm 
not sure that the FSF guidelines permit you to accept that patch from 
me, as I do not have an assignment on file for the gdb project.  This 
second patch is trivial in concept and obvious in implementation, but 
changes a LOT more than a dozen lines.

However, I release the two scripts above to the public domain, and they 
can be used by anyone for any purpose.  Have fun.

  auxv.c                      |    1 -
  bsd-kvm.c                   |    1 -
  cli/cli-cmds.c              |    1 -
  core-regset.c               |    1 -
  corefile.c                  |    1 -
  corelow.c                   |    4 ----
  dbxread.c                   |    1 -
  dwarf2read.c                |    1 -
  dwarfread.c                 |    1 -
  exec.c                      |    4 ----
  gdbtk/generic/gdbtk-cmds.c  |    1 -
  gdbtk/generic/gdbtk-hooks.c |    1 -
  gdbtk/generic/gdbtk.c       |    1 -
  go32-nat.c                  |    1 -
  hpux-thread.c               |    1 -
  i386v-nat.c                 |    1 -
  inflow.c                    |    1 -
  inftarg.c                   |    1 -
  linux-nat.c                 |    1 -
  m68klinux-nat.c             |    1 -
  nto-procfs.c                |    1 -
  objfiles.c                  |    1 -
  ocd.c                       |    1 -
  ppc-bdm.c                   |    1 -
  ppc-linux-nat.c             |    1 -
  proc-api.c                  |    1 -
  procfs.c                    |    1 -
  remote-fileio.c             |    1 -
  remote-rdp.c                |    4 ----
  remote-sds.c                |    1 -
  remote-sim.c                |    1 -
  remote.c                    |    1 -
  rs6000-nat.c                |    1 -
  ser-pipe.c                  |    1 -
  ser-unix.c                  |    1 -
  sol-thread.c                |    1 -
  solib-aix5.c                |    1 -
  solib-sunos.c               |    1 -
  source.c                    |    4 ----
  symfile.c                   |    4 ----
  symtab.c                    |    1 -
  tui/tui-hooks.c             |    1 -
  tui/tui-io.c                |    1 -
  tui/tui.c                   |    1 -
  uw-thread.c                 |    1 -
  win32-nat.c                 |    1 -
  wince.c                     |    1 -
  xcoffread.c                 |    1 -
  48 files changed, 63 deletions(-)

2006-02-21  Charles Wilson  <cygwin@...>

	* gdb/auxv.c: Remove <fcntl.h>
	* gdb/bsd-kvm.c: Remove <fcntl.h>
	* gdb/cli/cli-cmds.c: Remove <fcntl.h>
	* gdb/core-regset.c: Remove <fcntl.h>
	* gdb/corefile.c: Remove <fcntl.h>
	* gdb/corelow.c: Remove <fcntl.h>. Remove O_BINARY
	macro definition.
	* gdb/dbxread.c: Remove <fcntl.h>
	* gdb/dwarf2read.c: Remove <fcntl.h>
	* gdb/dwarfread.c: Remove <fcntl.h>
	* gdb/exec.c: Remove <fcntl.h>. Remove O_BINARY macro
	definition
	* gdb/gdbtk/generic/gdbtk-cmds.c: Remove <fcntl.h>
	* gdb/gdbtk/generic/gdbtk-hooks.c: Remove <fcntl.h>
	* gdb/gdbtk/generic/gdbtk.c: Remove <fcntl.h>
	* gdb/go32-nat.c: Remove <fcntl.h>
	* gdb/hpux-thread.c: Remove <fcntl.h>
	* gdb/i386v-nat.c: Remove <fcntl.h>
	* gdb/inflow.c: Remove <fcntl.h>
	* gdb/inftarg.c: Remove <fcntl.h>
	* gdb/linux-nat.c: Remove <fcntl.h>
	* gdb/m68klinux-nat.c: Remove <fcntl.h>
	* gdb/nto-procfs.c: Remove <fcntl.h>
	* gdb/objfiles.c: Remove <fcntl.h>
	* gdb/ocd.c: Remove <fcntl.h>
	* gdb/ppc-bdm.c: Remove <fcntl.h>
	* gdb/ppc-linux-nat.c: Remove <fcntl.h>
	* gdb/proc-api.c: Remove <fcntl.h>
	* gdb/procfs.c: Remove <fcntl.h>
	* gdb/remote-fileio.c: Remove <fcntl.h>
	* gdb/remote-rdp.c: Remove <fcntl.h>. Remove O_BINARY
	macro definition
	* gdb/remote-sds.c: Remove <fcntl.h>
	* gdb/remote-sim.c: Remove <fcntl.h>
	* gdb/remote.c: Remove <fcntl.h>
	* gdb/rs6000-nat.c: Remove <fcntl.h>
	* gdb/ser-pipe.c: Remove <fcntl.h>
	* gdb/ser-unix.c: Remove <fcntl.h>
	* gdb/sol-thread.c: Remove <fcntl.h>
	* gdb/solib-aix5.c: Remove <fcntl.h>
	* gdb/solib-sunos.c: Remove <fcntl.h>
	* gdb/source.c: Remove <fcntl.h>. Remove O_BINARY
	macro definition
	* gdb/symfile.c: Remove <fcntl.h>. Remove O_BINARY
	macro definition
	* gdb/symtab.c: Remove <fcntl.h>
	* gdb/tui/tui-hooks.c: Remove <fcntl.h>
	* gdb/tui/tui-io.c: Remove <fcntl.h>
	* gdb/tui/tui.c: Remove <fcntl.h>
	* gdb/uw-thread.c: Remove <fcntl.h>
	* gdb/win32-nat.c: Remove <fcntl.h>
	* gdb/wince.c: Remove <fcntl.h>
	* gdb/xcoffread.c: Remove <fcntl.h>

NOTE: the following 9 files all #include <fcntl.h> but do NOT #include 
defs.h -- so I didn't (and the scripts do not) modify them.  None of 
these 9 files use the O_BINARY symbol, either.

   gdb/terminal.h
   gdb/gdbserver/gdbreplay.c
   gdb/gdbserver/linux-low.c
   gdb/gdbserver/remote-utils.c
   gdb/gdbserver/terminal.h
   gdb/testsuite/gdb.base/bigcore.c
   gdb/testsuite/gdb.base/coremaker.c
   gdb/testsuite/gdb.base/fileio.c
   gdb/testsuite/gdb.hp/gdb.base-hp/genso-thresh.c

--
Chuck
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: gdb-6.4.50.20060131-cvs.solib.patch-attempt2-part1
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20060222/0214f7db/attachment.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: gdb-6.4.50.20060131-cvs.solib.patch-attempt2-part2
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20060222/0214f7db/attachment-0001.ksh>


More information about the Gdb-patches mailing list