Re: PATCH: Avoid accidentally opening files for write

[Mark Mitchell <mark@codesourcery.com>]

Nick Clifton wrote:
> Hi Mark,
> 
>> Tested on x86_64-unknown-linux-gnu.  OK to apply?
> 
> 
> Have you tested this patch in both POSIX and non-POSIX build 
> environments ?  

I did not test in a non-POSIX *build* environment, but I did test in a 
non-POIX *target* environment.  In particular, I built a MinGW GDB using 
a GNU/Linux host, and verified that this fixed the bug.  (I used an 
older version of GDB that still had a tendency to mangle its output; 
using this patch stopped it from doing the mangling because it no longer 
tried to write out the file.)

 > assume so, but it would be nice to have that confirmed.  Also with a
> 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.  Just a 
build, or should I try to run some kind of testsuite as well?  (Is there 
a way to run the binutils testuite on all targets all at once?)

>> !   if (strchr (mode, '+'))
>> !     nbfd->direction = both_direction;
>> !   else if (strchr (mode, 'r'))
>> !     nbfd->direction = read_direction;
>> !   else
>> !     nbfd->direction = write_direction;
> 
> 
> This assumes that the contents of 'mode' are well defined.  Is this the 
> case for non-POSIX environments ?  For example can we be sure that the 
> character 'R' is never used to indicate read-only status, or that an OS 
> might allow a file created with just "a" to have the newly-written-to 
> parts read back, effectively making "a" a read-and-write mode ?  What I 
> am getting at is, should bfd_fopen() take an explicit extra parameter 
> which tells BFD whether this file is intended for reading, writing or 
> both ?

Hmm.  In practice, we always use one of the FOPEN_* macros as an 
argument, and these do follow the rules implied by what I wrote.  But, 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.  OK to make that change before check-in, 
or would you like me to resubmit?

I'm not aware of OSes that do as you say, but, in any case, I don't 
think we need to worry about OSes that accept other variations.  Clients 
of BFD should be using the standard syntax.  It's OK if they use OS 
extensions, but I think it's reasonable to say that if they mean "read" 
they use "r" and not "R".

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