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"


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


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