Re: PATCH: Avoid accidentally opening files for write

Mark Mitchell
2005-06-07T23:02:52Z

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