Re: [patch, libgfortran] PR 31915 - fix reading/writing of real(10) variables with convert="SWAP"

[Tobias Burnus <burnus at net-b dot de>]

Hi Janne,

Janne Blomqvist wrote:
> Tobias Burnus wrote:
>> Updated patch below. Regression test is still running, but assuming it
>> succeeded, is the patch ok for trunk and 4.2.1?   
> You should fix the comments a few lines above your changes about the
> padding, which are no longer correct with your patch.
Thanks, I somehow missed them.


> That being said, I'm not convinced this is the right approach. We
> waste disk BW and space by including the pad bits; using the kind and
> not the size was IIRC a conscious decision back when the endian
> conversion patch was made. For "normal" situations this is justified,
> since that allows us to transfer lots of elements from an array in one
> go, but with endian conversion, we have to do it element by element
> anyways.
But if one does include the padding with convert=NATIVE on e.g. a
little-endian machine, one also need to do it when reading the same file
on a big-endian machine with convert=BIG_ENDIAN, or not? (Assuming that
sizeof(real(10) is the same on both platform.)

> (with the caveat that other platforms may pad differently, as I
> mentioned in my email linked above)
Well, fortunately, real(10) is only rarely used and "convert" is also
rarely used which means that it this padding problem does not affect
many users.

> Additionally taking into account that it does fix a bug, I guess it's
> ok for trunk (with the comment fixes).
Committed patch attached.

> Wait for a week or so before committing to the 4.2 branch to give
> people on non-Linux/x86 platforms time to test.
Well, I have to wait anyhow until the branch opens again. Currently it
is still frozen, I think. (Or not? It has been tagged and the FTP files
are released already.) In addition, I better wait until Dominique's
testsuite patch is in, otherwise testing data is missing too.

Tobias

Index: libgfortran/ChangeLog
===================================================================
--- libgfortran/ChangeLog	(Revision 124740)
+++ libgfortran/ChangeLog	(Arbeitskopie)
@@ -1,3 +1,9 @@
+2007-05-15  Tobias Burnus  <burnus@net-b.de>
+
+	PR libfortran/31915
+	* io/transfer.c (unformatted_read): Use proper size for real(10).
+	  (unformatted_write): Ditto.
+
 2007-05-14  Francois-Xavier Coudert  <fxcoudert@gcc.gnu.org>
 
 	PR fortran/30723
Index: libgfortran/io/transfer.c
===================================================================
--- libgfortran/io/transfer.c	(Revision 124740)
+++ libgfortran/io/transfer.c	(Arbeitskopie)
@@ -721,12 +721,13 @@
       p = dest;
       
       /* By now, all complex variables have been split into their
-	 constituent reals.  For types with padding, we only need to
-	 read kind bytes.  We don't care about the contents
-	 of the padding.  If we hit a short record, then sz is
-	 adjusted accordingly, making later reads no-ops.  */
+	 constituent reals.  */
       
-      sz = kind;
+      if (type == BT_REAL || type == BT_COMPLEX)
+	sz = size_from_real_kind (kind);
+      else
+	sz = kind;
+
       for (i=0; i<nelems; i++)
 	{
  	  read_block_direct (dtp, buffer, &sz);
@@ -767,11 +768,13 @@
       p = source;
 
       /* By now, all complex variables have been split into their
-	 constituent reals.  For types with padding, we only need to
-	 read kind bytes.  We don't care about the contents
-	 of the padding.  */
+	 constituent reals.  */
 
-      sz = kind;
+      if (type == BT_REAL || type == BT_COMPLEX)
+	sz = size_from_real_kind (kind);
+      else
+	sz = kind;
+
       for (i=0; i<nelems; i++)
 	{
 	  reverse_memcpy(buffer, p, size);