This is the mail archive of the fortran@gcc.gnu.org mailing list for the GNU Fortran project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[gfortran, committed] Minor cleanup to find_substring_ref



Hi,


looking over find_substring_ref it seemed to me that it wouldn't work for embedded NULs until I realized that the character constant is already copied in gfc_copy_expr, and the call to strncpy() therefore is just repeating work done before. OTOH this means that the newly created string will occupy more meory than necessary.

This small patch fixes that by explicitly allocating a new chunk of memory of the right size. I've also added a testcase that verifies that everything goes right. While I was at this, I noticed that dump-parse-tree doesn't do the right thing for non-printable characters in strings, which I fixed.

I've committed this under the obviously correct rule as r128027 after the obligatory testing on i386-darwin.

Cheers,
- Tobi
Index: gcc/testsuite/ChangeLog
===================================================================
--- gcc/testsuite/ChangeLog	(revision 128023)
+++ gcc/testsuite/ChangeLog	(working copy)
@@ -1,3 +1,7 @@
+2007-09-02  Tobias Schlüuter  <tobi@gcc.gnu.org>
+
+	* gfortran.dg/substr_6.f90: New test.
+
 2007-09-02  Joseph Myers  <joseph@codesourcery.com>
 
 	PR middle-end/33272
Index: gcc/testsuite/gfortran.dg/substr_6.f90
===================================================================
--- gcc/testsuite/gfortran.dg/substr_6.f90	(revision 0)
+++ gcc/testsuite/gfortran.dg/substr_6.f90	(revision 0)
@@ -0,0 +1,14 @@
+! { dg-do run }
+! Check that NULs don't mess up constant substring simplification
+CHARACTER(5), parameter :: c0(1) = (/ "123" // ACHAR(0) // "5" /)
+CHARACTER*5 c(1)
+CHARACTER(1), parameter :: c1(5) = (/ "1", "2", "3", ACHAR(0), "5" /)
+
+c = c0(1)(-5:-8)
+if (c(1) /= "     ") call abort()
+c = (/ c0(1)(1:5) /)
+do i=1,5
+   if (c(1)(i:i) /= c1(i)) call abort()
+end do
+print *, c(1)
+end
Index: gcc/fortran/dump-parse-tree.c
===================================================================
--- gcc/fortran/dump-parse-tree.c	(revision 128023)
+++ gcc/fortran/dump-parse-tree.c	(working copy)
@@ -290,6 +290,28 @@ gfc_show_constructor (gfc_constructor *c
 }
 
 
+static void
+show_char_const (const char *c, int length)
+{
+  int i;
+
+  gfc_status_char ('\'');
+  for (i = 0; i < length; i++)
+    {
+      if (c[i] == '\'')
+	gfc_status ("''");
+      else if (ISPRINT (c[i]))
+	gfc_status_char (c[i]);
+      else
+	{
+	  gfc_status ("' // ACHAR(");
+	  printf ("%d", c[i]);
+	  gfc_status (") // '");
+	}
+    }
+  gfc_status_char ('\'');
+}
+
 /* Show an expression.  */
 
 void
@@ -307,16 +329,7 @@ gfc_show_expr (gfc_expr *p)
   switch (p->expr_type)
     {
     case EXPR_SUBSTRING:
-      c = p->value.character.string;
-
-      for (i = 0; i < p->value.character.length; i++, c++)
-	{
-	  if (*c == '\'')
-	    gfc_status ("''");
-	  else
-	    gfc_status ("%c", *c);
-	}
-
+      show_char_const (p->value.character.string, p->value.character.length);
       gfc_show_ref (p->ref);
       break;
 
@@ -362,20 +375,8 @@ gfc_show_expr (gfc_expr *p)
 	  break;
 
 	case BT_CHARACTER:
-	  c = p->value.character.string;
-
-	  gfc_status_char ('\'');
-
-	  for (i = 0; i < p->value.character.length; i++, c++)
-	    {
-	      if (*c == '\'')
-		gfc_status ("''");
-	      else
-		gfc_status_char (*c);
-	    }
-
-	  gfc_status_char ('\'');
-
+	  show_char_const (p->value.character.string, 
+			   p->value.character.length);
 	  break;
 
 	case BT_COMPLEX:
Index: gcc/fortran/ChangeLog
===================================================================
--- gcc/fortran/ChangeLog	(revision 128023)
+++ gcc/fortran/ChangeLog	(working copy)
@@ -1,3 +1,10 @@
+2007-09-02  Tobias Schlüuter  <tobi@gcc.gnu.org>
+
+	* dump-parse-tree.c (show_char_const): New function.
+	(gfc_show_expr): Use it.
+	* expr.c (find_substring_ref): Rework to not keep characters
+	dangling beyond end of string.
+
 2007-08-31  Tobias Burnus  <burnus@net-b.de>
 
 	PR fortran/33232
Index: gcc/fortran/expr.c
===================================================================
--- gcc/fortran/expr.c	(revision 128023)
+++ gcc/fortran/expr.c	(working copy)
@@ -1329,6 +1329,7 @@ find_substring_ref (gfc_expr *p, gfc_exp
 {
   int end;
   int start;
+  int length;
   char *chr;
 
   if (p->ref->u.ss.start->expr_type != EXPR_CONSTANT
@@ -1336,13 +1337,16 @@ find_substring_ref (gfc_expr *p, gfc_exp
     return FAILURE;
 
   *newp = gfc_copy_expr (p);
-  chr = p->value.character.string;
+  gfc_free ((*newp)->value.character.string);
+
   end = (int) mpz_get_ui (p->ref->u.ss.end->value.integer);
   start = (int) mpz_get_ui (p->ref->u.ss.start->value.integer);
+  length = end - start + 1;
 
-  (*newp)->value.character.length = end - start + 1;
-  strncpy ((*newp)->value.character.string, &chr[start - 1],
-	   (*newp)->value.character.length);
+  chr = (*newp)->value.character.string = gfc_getmem (length + 1);
+  (*newp)->value.character.length = length;
+  memcpy (chr, &p->value.character.string[start - 1], length);
+  chr[length] = '\0';
   return SUCCESS;
 }
 

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]