[PATCH, libgfortran] PR25039 Comma short circuit read field

[Jerry DeLisle]

Paul Thomas wrote:
> 
> :REVIEWMAIL:
> 
> Jerry,
> 
>> Bootstrapped and regression tested.  OK for 4.2 and 4.1?
> 
> 
> It's a good idea for you to record, for posternity, on what you 
> committed these acts.
> 
> For the record, I have bubblestrapped and regtested your patch on 
> FC3/Athlon1700 and it works.
> 
>> Index: io/read.c
>>
>> ===================================================================
>> --- io/read.c    (revision 108010)
>> +++ io/read.c    (working copy)
>> @@ -244,7 +244,9 @@ read_a (st_parameter_dt *dtp, const fnod
>>   if (w == -1) /* '(A)' edit descriptor  */
>>     w = length;
>>  
>>
> Would you be so kind, since I know that you work on FC4, as to go visit 
> the Wiki and find out about setting up svn to yield context diffs?   
> Also, as a convenience for the reviewer, please make the diff relative 
> to trunk/gcc-4_x-branch.
> 
> Apart from this, the patch looks fine.  It certainly fixes the PR!
> 
>>      write(10,'(a)') "1,2       35"
>>      rewind(10)
>>      read(10,'(3i9)') i1,i2,i3
>>      if(i1.ne.1) call abort()
>>      if(i2.ne.23) call abort()
>>  
>>
> Is this I9 with 7 spaces really correct/desirable?  It does not check 
> the comma short-circuiting, does it?

Normally the read would see that first comma in the file while it is expecting a 
digit and error out.  So I am checking two things here, that reading into i1 
stops at the comma and the next record for i2 starts just after the comma.  Then 
in the reading of i2, I am checking that the spaces are interpreted as NULLs. (I 
am doing that because the default NULL settings on the files were messed up and 
I was fixing that at the time too)

> 
>>      if(i3.ne.5) call abort()
>>      rewind(10)
>>      write(10,'(a)') "1.1,1.2     1.3"
>>      rewind(10)
>>      read(10,'(3f9.2)') r1,r2,r3
>>      if(r1.ne.1.1) call abort()
>>      if(r2.ne.1.21) call abort()
>>  
>>
> ditto....

similarly as above, checking NULL spaces.

> 
> What are the issues in respect of your patch, versus that which Iwan 
> proposed?

The only issue is that I did not see Iwan's patch until after I did mine.  His 
patch is fine, though I don't think it is necessary to reset the eor flag as he 
did since the eor condition gets caught above this code.  Iwan's patch adds an 
argument to read_block, which is Ok.  When I was thinking about it at the time I 
thought I would just add a flag bit, instead.  I have also added in the 
std_notify code since this is outside of F95 standard behavior.

The only other issue is copyright assignment.  Since I did not know if Iwan had 
this yet, I thought it best to go with my patch so that this PR does not linger 
waiting for the process to complete.  (Iwan, is it your intent to be a 
contributor? You are certainly welcome to, just need the paperwork in place.  By 
no means is there any intent to slight your effort.)

   By the way, welcome, Iwan.  Please feel free to propose
> further patces/improvements to gfortran and get your paperwork sorted 
> out, if you have not done already!
> 
> Modulo a reply to the last query, this is OK for committing on trunk and 
> 4.1.
> 
> Best regards
> 
> Paul
> 
>