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

[Janne Blomqvist]

On 7/3/07, Jerry DeLisle <jvdelisle@verizon.net> wrote:
> 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.

Right. So the segfault was sort of due to freeing it too early?

> > 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.

I see. Sorry about the noise.

> > 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?

Well my understading was that one is supposed to be using unified diffs. :)

But now that I check (http://gcc.gnu.org/contribute.html) it seems
that both are acceptable. Though I personally do prefer unified diffs.

-- 
Janne Blomqvist