RFR: 8214776: Avoid GCC 8.X strncpy() errors in JFR code

[Simon Tooke]

On 2018-12-11 11:06 a.m., Simon Tooke wrote:
> On 2018-12-11 8:47 a.m., Severin Gehwolf wrote:
>> Hi Simon,
>>
>> On Mon, 2018-12-10 at 10:25 -0500, Simon Tooke wrote:
>>> This small patch fixes some simple warnings in JFR code, found by GCC 8.1
>>> 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)
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8214776
>>> Webrev:
>>> http://cr.openjdk.java.net/~sgehwolf/webrevs/stooke/JDK-8214776/02/webrev/

Revised webrev:

http://cr.openjdk.java.net/~sgehwolf/webrevs/stooke/JDK-8214776/03/webrev/

Thanks again to Severin


>> Shouldn't these:
>>
>> @@ -332,7 +331,7 @@
>>      if (NULL == emergency_dump_path) {
>>        return NULL;
>>      }
>> -    strncpy(emergency_dump_path, buffer, emergency_filename_length);
>> +    strncpy(emergency_dump_path, buffer, emergency_filename_length + 1);
>>      emergency_dump_path[emergency_filename_length] = '\0';
>>    }
>>    return emergency_dump_path;
>> @@ -407,7 +406,7 @@
>>    if (_path == NULL) {
>>      return false;
>>    }
>> -  strncpy(_path, path, path_len);
>> +  strncpy(_path, path, path_len + 1);
>>    _path[path_len] = '\0';
>>
>> Be this instead?
>>
>> @@ -332,8 +331,7 @@
>>      if (NULL == emergency_dump_path) {
>>        return NULL;
>>      }
>> -    strncpy(emergency_dump_path, buffer, emergency_filename_length);
>> -    emergency_dump_path[emergency_filename_length] = '\0';
>> +    strncpy(emergency_dump_path, buffer, emergency_filename_length + 1);
>>    }
>>    return emergency_dump_path;
>>  }
>> @@ -407,8 +405,7 @@
>>    if (_path == NULL) {
>>      return false;
>>    }
>> -  strncpy(_path, path, path_len);
>> -  _path[path_len] = '\0';
>> +  strncpy(_path, path, path_len + 1);
> Yes, you're absolutely right - the null terminator is now set in the
> line before.
> I can produce a new webrev.
>
>> Thanks,
>> Severin
>>
> Thank you,
> -Simon
>
>