[12] RFR: 8214777: Avoid some GCC 8.X strncpy() errors in HotSpot

[Simon Tooke]

On 2018-12-10 4:47 p.m., David Holmes wrote:
> Hi Simon,
Hello, and thanks for the review!
>
> On 11/12/2018 4:57 am, Simon Tooke wrote:
>> This small patch fixes some simple warnings in Hotspot code, found by
>> GCC 8.1
>
> I hate having to change code just to silence faulty tools :(
I do too, but I hate disabling warnings globally more.
(I'm certainly open to skipping GCC 8.1 because of known false positives,
and I note that xmpstream.cpp already has some Clang warning disabled)

>
>> Essentially, any code sequence of the pattern
>>
>>      int l = strlen(somestring)
>>      char* buffer = malloc(l + 1)
>>      strncpy(buffer, somestring, l)
>>      buffer[l] = 0
>>
>> is replaced by
>>
>>      int len = strlen(somestring)
>>      char* buffer = malloc(len + 1)
>>      strncpy(buffer, somestring, len + 1)
>>
>> For xmlstream.cpp, this is actually a small inefficiency, as the null
>> byte is immediately overwritten; but it makes GCC happy.
>
> For xmlstream and src/hotspot/share/runtime/arguments.cpp I can't
> convince myself that the change is always correct, because (taking the
> arguments example) I'm not sure what "pos-tail" evaluates to and
> exactly what the string might look like, and so whether we're
> guaranteed to hit a Nul-terminator in that case.
>
>        if(tail != NULL) {
>           const char* pos = strchr(tail, '=');
>           size_t len = (pos == NULL) ? strlen(tail) : pos - tail;
> !         char* name = strncpy(NEW_C_HEAP_ARRAY(char, len + 1,
> mtArguments), tail, len + 1);

I agree with you on arguments.cpp - the '\0' assignment should be restored.
On xmlstream.cpp, I believe it's fine, as the last character is
immediately overwritten by the following strcpy(), so it is irrelevant
what was there.

>
> Aside: note this bug is currently targeted to 13, if it may be pushed
> to 12 it needs to be updated first.
Noted.
>
> Thanks,
> David
> -----
Thank you again.
>
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8214777
>> Webrev:
>> http://cr.openjdk.java.net/~sgehwolf/webrevs/stooke/JDK-8214777/02/webrev/
>>
>>