Re: [patch, libfortran] Bounds checking for (c|eo)shift, bounds checking library functions

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

Thomas Koenig schrieb:
> this patch implements bounds checking for the cshift and eoshift library
> functions.
> Regression-tested on i686-pc-linux-gnu.  OK for trunk?
>   

+extern void bounds_equal_extents (array_t *, array_t *, const char *,
+				  const char *);
+internal_proto(bounds_equal_extents);
+
+void bounds_reduced_extents (array_t *, array_t *, int, const char *,
+			     const char *intrinsic);
+internal_proto(bounds_reduced_extents);


Is there a reason that the first one is extern and the second one is not?



+This file is part of the GNU Fortran 95 runtime library (libgfortran).

Remove the "95".




+/* Bounds checking for the return values of the iforeach functions.  */

and

+/* Check the return of functions generated from ifunction.m4.  */


Can you provide a short comment what the function actually does? For the
next one you do so properly:

+/* Check that two arrays have equal extents, or are both zero-sized.  Abort
+   with a runtime error if this is not the case.  Complain that a has the
+   wrong size.  */
+



+    runtime_error ("rank of return array in %s intrinsic"
+		   " should be 1, is %ld", name, (long int) ret_rank);


Start with a capital letter. I somehow have parsing problems with ", is
%ld" [contrary to another message below - "...: is %ld, should be %ld" -
maybe due to the colon?) Thus I am tempted to insert either a "but" or
an "it".


+    runtime_error ("Incorrect extent in return value of"
+		   " %s intrnisic: is %ld, should be %ld",


Typo: intrinsic


@@ -61,6 +62,8 @@ eoshift0 (gfc_array_char * ret, const gf
   soffset = 0;
   roffset = 0;
 
+  arraysize = size0 ((array_t *) array);
+
   if (ret->data == NULL)
     {
       int i;
@@ -83,13 +86,22 @@ eoshift0 (gfc_array_char * ret, const gf
 	  GFC_DIMENSION_SET(ret->dim[i], 0, ub, str);
 
         }
+
+      if (arraysize > 0)
+	ret->data = internal_malloc_size (size * arraysize);
+      else
+	ret->data = internal_malloc_size (1);
+
     }



Sorry, I do not understand what you are doing here.  For completeness,
the line after "int i" is:
      ret->data = internal_malloc_size (size * size0 ((array_t *)array));
Shouldn't you move the "if(arraysize > 0) block up to that place and
replace the line by the if-else block?

Otherwise the patch looks OK.

Tobias