Re: PATCH: Avoid accidentally opening files for write

[Mark Mitchell <mark@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 899 bytes --]

Nick Clifton wrote:
> Hi Mark,
> 
>>> patch to a generic part of BFD it would be good if you could also 
>>> test with a --enable-targets=all build, just to make sure.
> 
> 
>> I didn't know about that, but will try it before check-in. 

It worked -- except for a warning from the new ms1 port when compiling 
readelf.  I worked around that, and committed anyhow, as that was an 
unrelated issue.

>> I could tighten the test to check for just what ISO C requires, which 
>> is that the characters must occur at the start of the string, so using 
>> strchr is probably incorrect.
> 
> No please just make the change before check-in.

I've attached the version committed.  It's now on both mainline and 
csl-arm-20050325-branch.  Thanks for the quick review!

GDB folks, may I now commit the other half of the patch?

Thanks,

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

[-- Attachment #2: bfd.fopen.patch --]
[-- Type: text/plain, Size: 8262 bytes --]

2005-06-06  Mark Mitchell  <mark@codesourcery.com>

	* opncls.c (bfd_fopen): New API.
	(bfd_openr): Use it.
	(bfd_fdopenr): Likewise.
	* bfd-in2.h: Regenerated.

Index: bfd-in2.h
===================================================================
RCS file: /cvs/src/src/bfd/bfd-in2.h,v
retrieving revision 1.344
diff -c -5 -p -r1.344 bfd-in2.h
*** bfd-in2.h	7 Jun 2005 21:07:29 -0000	1.344
--- bfd-in2.h	7 Jun 2005 22:52:03 -0000
*************** extern struct coff_comdat_info *bfd_coff
*** 891,900 ****
--- 891,903 ----
  
  /* Extracted from init.c.  */
  void bfd_init (void);
  
  /* Extracted from opncls.c.  */
+ bfd *bfd_fopen (const char *filename, const char *target,
+     const char *mode, int fd);
+ 
  bfd *bfd_openr (const char *filename, const char *target);
  
  bfd *bfd_fdopenr (const char *filename, const char *target, int fd);
  
  bfd *bfd_openstreamr (const char *, const char *, void *);
Index: opncls.c
===================================================================
RCS file: /cvs/src/src/bfd/opncls.c,v
retrieving revision 1.33
diff -c -5 -p -r1.33 opncls.c
*** opncls.c	4 May 2005 15:53:36 -0000	1.33
--- opncls.c	7 Jun 2005 22:52:03 -0000
*************** SECTION
*** 126,183 ****
  
  */
  
  /*
  FUNCTION
! 	bfd_openr
  
  SYNOPSIS
! 	bfd *bfd_openr (const char *filename, const char *target);
  
  DESCRIPTION
! 	Open the file @var{filename} (using <<fopen>>) with the target
! 	@var{target}.  Return a pointer to the created BFD.
  
  	Calls <<bfd_find_target>>, so @var{target} is interpreted as by
  	that function.
  
  	If <<NULL>> is returned then an error has occured.   Possible errors
  	are <<bfd_error_no_memory>>, <<bfd_error_invalid_target>> or
  	<<system_call>> error.
  */
  
  bfd *
! bfd_openr (const char *filename, const char *target)
  {
    bfd *nbfd;
    const bfd_target *target_vec;
  
    nbfd = _bfd_new_bfd ();
    if (nbfd == NULL)
      return NULL;
  
    target_vec = bfd_find_target (target, nbfd);
    if (target_vec == NULL)
      {
        _bfd_delete_bfd (nbfd);
        return NULL;
      }
  
    nbfd->filename = filename;
-   nbfd->direction = read_direction;
  
!   if (bfd_open_file (nbfd) == NULL)
      {
-       /* File didn't exist, or some such.  */
-       bfd_set_error (bfd_error_system_call);
        _bfd_delete_bfd (nbfd);
        return NULL;
      }
  
    return nbfd;
  }
  
  /* Don't try to `optimize' this function:
  
     o - We lock using stack space so that interrupting the locking
         won't cause a storage leak.
     o - We open the file stream last, since we don't want to have to
--- 126,235 ----
  
  */
  
  /*
  FUNCTION
! 	bfd_fopen
  
  SYNOPSIS
! 	bfd *bfd_fopen (const char *filename, const char *target,
!                         const char *mode, int fd);
  
  DESCRIPTION
! 	Open the file @var{filename} with the target @var{target}.
! 	Return a pointer to the created BFD.  If @var{fd} is not -1,
! 	then <<fdopen>> is used to open the file; otherwise, <<fopen>>
! 	is used.  @var{mode} is passed directly to <<fopen>> or
! 	<<fdopen>>. 
  
  	Calls <<bfd_find_target>>, so @var{target} is interpreted as by
  	that function.
  
  	If <<NULL>> is returned then an error has occured.   Possible errors
  	are <<bfd_error_no_memory>>, <<bfd_error_invalid_target>> or
  	<<system_call>> error.
  */
  
  bfd *
! bfd_fopen (const char *filename, const char *target, const char *mode, int fd)
  {
    bfd *nbfd;
    const bfd_target *target_vec;
  
+   bfd_set_error (bfd_error_system_call);
+ 
    nbfd = _bfd_new_bfd ();
    if (nbfd == NULL)
      return NULL;
  
    target_vec = bfd_find_target (target, nbfd);
    if (target_vec == NULL)
      {
        _bfd_delete_bfd (nbfd);
        return NULL;
      }
+   
+ #ifdef HAVE_FDOPEN
+   if (fd != -1)
+     nbfd->iostream = fdopen (fd, mode);
+   else
+ #endif
+     nbfd->iostream = fopen (filename, mode);
+   if (nbfd->iostream == NULL)
+     {
+       _bfd_delete_bfd (nbfd);
+       return NULL;
+     }
  
+   /* OK, put everything where it belongs.  */
    nbfd->filename = filename;
  
!   /* Figure out whether the user is opening the file for reading,
!      writing, or both, by looking at the MODE argument.  */
!   if ((mode[0] == 'r' || mode[0] == 'w' || mode[0] == 'a') 
!       && mode[1] == '+')
!     nbfd->direction = both_direction;
!   else if (mode[0] == 'r')
!     nbfd->direction = read_direction;
!   else
!     nbfd->direction = write_direction;
! 
!   if (! bfd_cache_init (nbfd))
      {
        _bfd_delete_bfd (nbfd);
        return NULL;
      }
+   nbfd->opened_once = TRUE;
  
    return nbfd;
  }
  
+ /*
+ FUNCTION
+ 	bfd_openr
+ 
+ SYNOPSIS
+ 	bfd *bfd_openr (const char *filename, const char *target);
+ 
+ DESCRIPTION
+ 	Open the file @var{filename} (using <<fopen>>) with the target
+ 	@var{target}.  Return a pointer to the created BFD.
+ 
+ 	Calls <<bfd_find_target>>, so @var{target} is interpreted as by
+ 	that function.
+ 
+ 	If <<NULL>> is returned then an error has occured.   Possible errors
+ 	are <<bfd_error_no_memory>>, <<bfd_error_invalid_target>> or
+ 	<<system_call>> error.
+ */
+ 
+ bfd *
+ bfd_openr (const char *filename, const char *target)
+ {
+   return bfd_fopen (filename, target, FOPEN_RB, -1);
+ }
+ 
  /* Don't try to `optimize' this function:
  
     o - We lock using stack space so that interrupting the locking
         won't cause a storage leak.
     o - We open the file stream last, since we don't want to have to
*************** DESCRIPTION
*** 210,285 ****
  */
  
  bfd *
  bfd_fdopenr (const char *filename, const char *target, int fd)
  {
!   bfd *nbfd;
!   const bfd_target *target_vec;
    int fdflags;
  
    bfd_set_error (bfd_error_system_call);
  #if ! defined(HAVE_FCNTL) || ! defined(F_GETFL)
!   fdflags = O_RDWR;			/* Assume full access.  */
  #else
    fdflags = fcntl (fd, F_GETFL, NULL);
- #endif
    if (fdflags == -1)
      return NULL;
  
-   nbfd = _bfd_new_bfd ();
-   if (nbfd == NULL)
-     return NULL;
- 
-   target_vec = bfd_find_target (target, nbfd);
-   if (target_vec == NULL)
-     {
-       _bfd_delete_bfd (nbfd);
-       return NULL;
-     }
- 
- #ifndef HAVE_FDOPEN
-   nbfd->iostream = fopen (filename, FOPEN_RB);
- #else
    /* (O_ACCMODE) parens are to avoid Ultrix header file bug.  */
    switch (fdflags & (O_ACCMODE))
      {
!     case O_RDONLY: nbfd->iostream = fdopen (fd, FOPEN_RB);   break;
!     case O_WRONLY: nbfd->iostream = fdopen (fd, FOPEN_RUB);  break;
!     case O_RDWR:   nbfd->iostream = fdopen (fd, FOPEN_RUB);  break;
      default: abort ();
      }
  #endif
  
!   if (nbfd->iostream == NULL)
!     {
!       _bfd_delete_bfd (nbfd);
!       return NULL;
!     }
! 
!   /* OK, put everything where it belongs.  */
!   nbfd->filename = filename;
! 
!   /* As a special case we allow a FD open for read/write to
!      be written through, although doing so requires that we end
!      the previous clause with a preposition.  */
!   /* (O_ACCMODE) parens are to avoid Ultrix header file bug.  */
!   switch (fdflags & (O_ACCMODE))
!     {
!     case O_RDONLY: nbfd->direction = read_direction; break;
!     case O_WRONLY: nbfd->direction = write_direction; break;
!     case O_RDWR: nbfd->direction = both_direction; break;
!     default: abort ();
!     }
! 
!   if (! bfd_cache_init (nbfd))
!     {
!       _bfd_delete_bfd (nbfd);
!       return NULL;
!     }
!   nbfd->opened_once = TRUE;
! 
!   return nbfd;
  }
  
  /*
  FUNCTION
  	bfd_openstreamr
--- 262,295 ----
  */
  
  bfd *
  bfd_fdopenr (const char *filename, const char *target, int fd)
  {
!   const char *mode;
! #if defined(HAVE_FCNTL) && defined(F_GETFL)
    int fdflags;
+ #endif
  
    bfd_set_error (bfd_error_system_call);
  #if ! defined(HAVE_FCNTL) || ! defined(F_GETFL)
!   mode = FOPEN_RUB; /* Assume full access.  */
  #else
    fdflags = fcntl (fd, F_GETFL, NULL);
    if (fdflags == -1)
      return NULL;
  
    /* (O_ACCMODE) parens are to avoid Ultrix header file bug.  */
    switch (fdflags & (O_ACCMODE))
      {
!     case O_RDONLY: mode = FOPEN_RB;
!     case O_WRONLY: mode = FOPEN_RUB;
!     case O_RDWR:   mode = FOPEN_RUB;
      default: abort ();
      }
  #endif
  
!   return bfd_fopen (filename, target, mode, fd);
  }
  
  /*
  FUNCTION
  	bfd_openstreamr