Patchwork [libgfortran] PR46733 fflush called when reading from a string

login
register
mail settings
Submitter Jerry DeLisle
Date Nov. 9, 2010, 1:54 p.m.
Message ID <4CD952A5.7070904@frontier.com>
Download mbox | patch
Permalink /patch/70545/
State New
Headers show

Comments

Jerry DeLisle - Nov. 9, 2010, 1:54 p.m.
Committed as obvious.

Sending        ChangeLog
Sending        io/transfer.c
Transmitting file data ..
Committed revision 166490.


Regards,

Jerry
Janne Blomqvist - Nov. 9, 2010, 6:37 p.m.
On Tue, Nov 9, 2010 at 15:54, Jerry DeLisle <jvdelisle@frontier.com> wrote:
> Committed as obvious.
>
> Sending        ChangeLog
> Sending        io/transfer.c
> Transmitting file data ..
> Committed revision 166490.
>
>
> Index: io/transfer.c
> ===================================================================
> --- io/transfer.c       (revision 166469)
> +++ io/transfer.c       (working copy)
> @@ -2646,7 +2646,8 @@
>     }
>
>   /* Bugware for badly written mixed C-Fortran I/O.  */
> -  flush_if_preconnected(dtp->u.p.current_unit->s);
> +  if (!is_internal_unit (dtp))
> +    flush_if_preconnected(dtp->u.p.current_unit->s);
>
>   dtp->u.p.current_unit->mode = dtp->u.p.mode;

Thanks for fixing this bug.

Generally though, I think this flush_if_preconnected() function should
be removed. Either we

1) Never buffer the preconnected units (or, we have mandatory line
buffering via the use of fbuf.h, but no more than that), in which case
we should remove GFORTRAN_UNBUFFERED_PRECONNECTED and modify the
checks in unix.c:fd_to_stream() such that we always call raw_init()
for preconnected units. This would be practically equivalent to the
status quo.

2) Potentially buffer preconnected units if they are redirected
to/from a regular file, and tell users to use GFORTRAN_UNBUFFERED_ALL
if they wish to prevent said buffering.

In neither of the above cases is flush_if_preconnected() needed. Any
opinions on which approach is preferred?
Jerry DeLisle - Nov. 9, 2010, 9:35 p.m.
On 11/09/2010 10:37 AM, Janne Blomqvist wrote:
> On Tue, Nov 9, 2010 at 15:54, Jerry DeLisle<jvdelisle@frontier.com>  wrote:
>> Committed as obvious.
>>
>> Sending        ChangeLog
>> Sending        io/transfer.c
>> Transmitting file data ..
>> Committed revision 166490.
>>
>>
>> Index: io/transfer.c
>> ===================================================================
>> --- io/transfer.c       (revision 166469)
>> +++ io/transfer.c       (working copy)
>> @@ -2646,7 +2646,8 @@
>>      }
>>
>>    /* Bugware for badly written mixed C-Fortran I/O.  */
>> -  flush_if_preconnected(dtp->u.p.current_unit->s);
>> +  if (!is_internal_unit (dtp))
>> +    flush_if_preconnected(dtp->u.p.current_unit->s);
>>
>>    dtp->u.p.current_unit->mode = dtp->u.p.mode;
>
> Thanks for fixing this bug.
>
> Generally though, I think this flush_if_preconnected() function should
> be removed. Either we
>
> 1) Never buffer the preconnected units (or, we have mandatory line
> buffering via the use of fbuf.h, but no more than that), in which case
> we should remove GFORTRAN_UNBUFFERED_PRECONNECTED and modify the
> checks in unix.c:fd_to_stream() such that we always call raw_init()
> for preconnected units. This would be practically equivalent to the
> status quo.
>
> 2) Potentially buffer preconnected units if they are redirected
> to/from a regular file, and tell users to use GFORTRAN_UNBUFFERED_ALL
> if they wish to prevent said buffering.
>
> In neither of the above cases is flush_if_preconnected() needed. Any
> opinions on which approach is preferred?
>
>
I agree we should get rid of it.  I am leaning a bit toward 1) above. Do you 
think we should wait until after 4.6 release?  After 4.6 release we plan to 
break ABI in a lot of places and this little tweak would be insignificant.  (Of 
course by itself it does not break ABI)

Jerry

Patch

Index: io/transfer.c
===================================================================
--- io/transfer.c	(revision 166469)
+++ io/transfer.c	(working copy)
@@ -2646,7 +2646,8 @@ 
      }

    /* Bugware for badly written mixed C-Fortran I/O.  */
-  flush_if_preconnected(dtp->u.p.current_unit->s);
+  if (!is_internal_unit (dtp))
+    flush_if_preconnected(dtp->u.p.current_unit->s);

    dtp->u.p.current_unit->mode = dtp->u.p.mode;