Re: [PATCH] Derive interface buffers from max name length

[Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>]

On December 3, 2015 10:46:09 AM GMT+01:00, Janne Blomqvist <blomqvist.janne@gmail.com> wrote:
>On Tue, Dec 1, 2015 at 6:51 PM, Bernhard Reutner-Fischer
><rep.dot.nop@gmail.com> wrote:
>> On 1 December 2015 at 15:52, Janne Blomqvist
><blomqvist.janne@gmail.com> wrote:
>>> On Tue, Dec 1, 2015 at 2:54 PM, Bernhard Reutner-Fischer
>>> <rep.dot.nop@gmail.com> wrote:
>>>> These three function used a hardcoded buffer of 100 but would be
>better
>>>> off to base off GFC_MAX_SYMBOL_LEN which denotes the maximum length
>of a
>>>> name in any of our supported standards (63 as of f2003 ff.).
>>>
>>> Please use xasprintf() instead (and free the result, or course). One
>>> of my backburner projects is to get rid of these static symbol
>>> buffers, and use dynamic buffers (or the symbol table) instead. We
>>> IIRC already have some ugly hacks by using hashing to get around
>>> GFC_MAX_SYMBOL_LEN when handling mangled symbols. Your patch doesn't
>>> make the situation worse per se, but if you're going to fix it, lets
>>> do it properly.
>>
>> I see.
>>
>> /scratch/src/gcc-6.0.mine/gcc/fortran$ git grep
>> "^[[:space:]]*char[[:space:]][[:space:]]*[^[;[:space:]]*\[" | wc -l
>> 142
>> /scratch/src/gcc-6.0.mine/gcc/fortran$ git grep "xasprintf" | wc -l
>> 32
>
>Yes, that's why it's on the TODO-list rather than on the DONE-list. :)
>
>> What about memory fragmentation when switching to heap-based
>allocation?
>> Or is there consensus that these are in the noise compared to other
>> parts of the compiler?
>
>Heap fragmentation is an issue, yes. I'm not sure it's that
>performance-critical, but I don't think there is any consensus. I just
>want to avoid ugly hacks like symbol hashing to fit within some fixed
>buffer. Perhaps an good compromise would be something like std::string
>with small string optimization, but as you have seen there is some
>resistance to C++. But this is more relevant for mangled symbols, so
>GFC_MAX_MANGLED_SYMBOL_LEN is more relevant here, and there's only a
>few of them left. So, well, if you're sure that mangled symbols are
>never copied into the buffers your patch modifies, please consider
>your original patch Ok as well. Whichever you prefer.
>
>Performance-wise I think a bigger benefit would be to use the symbol
>table more and then e.g. be able to do pointer comparisons rather than
>strcmp(). But that is certainly much more work.

Hm, worth a look indeed since that would certainly be a step in the right direction.

>
>> BTW:
>> $ git grep APO
>> io.c:  static const char *delim[] = { "APOSTROPHE", "QUOTE", "NONE",
>NULL };
>> io.c:  static const char *delim[] = { "APOSTROPHE", "QUOTE", "NONE",
>NULL };
>
>? What are you saying?

delim is duplicated, we should remove one instance.
thanks,