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]

Re: [patch, fortran] PR32432 SEGV/endless loop after: "ERROR: ... already is initialized"


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


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