This is the mail archive of the
fortran@gcc.gnu.org
mailing list for the GNU Fortran project.
Re: [patch, fortran] PR32432 SEGV/endless loop after: "ERROR: ... already is initialized"
- From: Jerry DeLisle <jvdelisle at verizon dot net>
- To: Janne Blomqvist <blomqvist dot janne at gmail dot com>
- Cc: Fortran List <fortran at gcc dot gnu dot org>, gcc-patches <gcc-patches at gcc dot gnu dot org>
- Date: Tue, 03 Jul 2007 06:46:42 -0700
- Subject: Re: [patch, fortran] PR32432 SEGV/endless loop after: "ERROR: ... already is initialized"
- References: <4689DD81.1090406@verizon.net> <7b446c2e0707030018n73e937c5ofa10e89e6bad0606@mail.gmail.com>
Janne Blomqvist wrote:
:REVIEWMAIL:
On 7/3/07, Jerry DeLisle <jvdelisle@verizon.net> wrote:
OK for trunk?
Yes. A few minor issues below though.
*************** gfc_assign_data_value (gfc_expr *lvalue,
*** 293,300 ****
gfc_error ("'%s' at %L already is initialized at %L",
lvalue->symtree->n.sym->name, &lvalue->where,
&init->where);
! gfc_free_expr (init);
! init = NULL;
}
if (init == NULL)
--- 295,301 ----
gfc_error ("'%s' at %L already is initialized at %L",
lvalue->symtree->n.sym->name, &lvalue->where,
&init->where);
! return FAILURE;
}
Won't this create a memory leak as you are no longer freeing init?
No, to enter into this condition, init is not NULL.
if (init == NULL)
expr = gfc_get_expr ();
else
expr = init;
So gfc_get_expr () is never called. expr is derived from the calling function.
This is why I believe the segfault was occurring.
*************** gfc_assign_data_value (gfc_expr *lvalue,
*** 423,428 ****
--- 424,431 ----
symbol->value = expr;
else
last_con->expr = expr;
+
+ return t;
}
Why not get rid of the t variable and just "return SUCCESS;", since
you never actually use the t variable for anything beyond initially
setting it to SUCCESS?
OK
Similarly in resolve.c,
! t = gfc_assign_data_value (var->expr, values.vnode->expr, offset);
! if (t == FAILURE)
! break;
Why not just
if (gfc_assign_data_value (...) == FAILURE)
break;
We have to break out of a loop and do some cleanup of mpz variables before
exiting and we must return t in this case. t is preset to SUCCESS.
Finally, please use unified diffs in the future; I suspect I'm not the
only one who has difficulties following context diffs.
I get this comment about unified diffs vs context diffs from time to time. It
was my understanding that context diffs are the gcc norm for patch submittals.
Maybe I have this misunderstood?
Thanks for review.
Regards,
Jerry