RFR (trivial): 8235819: -Wformat-overflow is reported from GCC 9

[Yasumasa Suenaga]

Hi Leo,

Thank you for your review.

On 2019/12/12 23:26, Leo Korinth wrote:
> Hi,
> 
> Your fix looks good to me (I am not a reviewer). Thanks for fixing this!
> 
> In the long run, I believe a better solution would be two have a separate function for doing the indentation so that one could use format strings to make the indentation level work for any level or, at least, assert on over-run. This is a good fix for now however.

Did you suggest like this?

   http://cr.openjdk.java.net/~ysuenaga/JDK-8235819/indent-function/

I agree with you basically, but this code is for logging. So I think it prefers simple.
In addition, we can catch the overrun via compiler warnings.

Indents[] is also used with literal in G1GCPhaseTimes::info_time() and more.
If we change Indents[] in future, they might be broken.
Of course we can add assert() to each codes, but it is strange, IMHO.

Thus I think we can take two approaches:

   1) Extends Indents[] if we need (webrev.00)
   2) Add a function to generate indent string (indent-function)

I prefer 1) at this time.


Thanks,

Yasumasa


> Thanks,
> Leo
> 
> 
> On 12/12/2019 13:51, Yasumasa Suenaga wrote:
>> Hi all,
>>
>> Please review this trivial change:
>>
>>    JBS: https://bugs.openjdk.java.net/browse/JDK-8235819
>>    webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8235819/webrev.00/
>>
>> I saw format-overflow warnings in g1GCPhaseTimes.cpp when I built jdk/jdk with GCC 9 on Fedora 31 as below:
>>
>> ------------------
>> /home/ysuenaga/OpenJDK/jdk/src/hotspot/share/gc/g1/g1GCPhaseTimes.cpp:319:17: error: '%s' directive argument is null [-Werror=format-overflow=]
>>    319 | out->print("%s", Indents[indent + 1]);
>>        | ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
>> ------------------
>>
>> Indents[] seems to be insufficient.
>> We need to give enough entries to it.
>>
>> This patch passed all tests on submit repo (mach5-one-ysuenaga-JDK-8235819-20191212-0925-7464553).
>>
>>
>> Thanks,
>>
>> Yasumasa