Re: [PATCH] PR fortran/24005

[Tobias Schlüter <tobias dot schlueter at physik dot uni-muenchen dot de>]

Steve Kargl wrote:
> In my debugging, only p->sym-module was NULL.  I'll happily
> remove these checks (although they can't hurt).

They can mask an error, which is very unlikely to ever occur.  After all, we
only build symbols, if we have a name for them.

>>Furthermore, the module name needn't be set, as interfaces can
>>reference symbols in the current scope,
>>IOW the correct check would be:
>>
>>if (strcmp (p->sym->name, q->sym->name) == 0
>>    && ((!p->sym->module && !q->sym->module)
> 
>            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
>>        || (p->sym->module && q->sym->module
> 
>               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Shouldn't we need only one of these?

No, because this catches the case that one is NULL while the other isn't.

>>            && strcmp (p->sym->module, q->sym->module) == 0)))
>>
>>NB I remember that we stumbled across a very similar bug when I
>>introduced the string table changes, which it turned out I had
>>missed, because glibc's strcmp does "the right thing" with NULL
>>arguments unlike FreeBSD's.
> 
> 
> I don't know the C standard requirements to argue for or against
> who is doing the right thing.  I do know that gfortran seg faults
> without my patch and runs fine with it.

IIRC the last time this came up we found out that FreeBSD is well within the
requirements of the C standard.  That's why I put "the right thing" in quotes.

>> Alternatively,
>>(if I understand C well enough) we could simply compare the string pointers
>>(they're allocated by gfc_get_string) like so:
>>
>>if (p->sym->name == q->sym->name && p->sym->module == q->sym->module)
> 
> 
> I'll try the above.  IIRC the string table stuff should contain 
> unique strings.

Ok.

Thanks,
- Tobi