RFR(XS): 8214707: Prevent GCC 8 from reporting error in ClassLoader::file_name_for_class_name()

[David Holmes]

On 7/12/2018 7:43 pm, Thomas Stüfe wrote:
> On Fri, Dec 7, 2018 at 2:09 AM David Holmes <david.holmes at oracle.com> wrote:
>>
>> +1
>>
>> Annoying to have to workaround a buggy compiler though. :(
>>
> 
> I do not think this is a compiler bug but a usage error which in this
> case was harmless.
> 
> I think the compiler warning is correct.

We replaced the third argument to strncpy with a local variable that has 
the exact same value as what was warned about. In other words we passed 
the exact same value - yet one case got a warning and the other did not.

Either the value is wrong or the value is right.

Cheers,
David
-----

> strncpy(dest, src, destlen)
> 
> destlen should be the size of the receiving buffer. It should not be
> the string len of the source string. This seems to be a common
> misunderstanding, I keep seeing this.
> 
> In this case, super-correct would be something like this:
> 
> size_t remaining = <buffer len>
> strncpy(file_name, class_name, remaining);
> size_t written = strlen(file_name)
> remaining -= written
> strncpy(file_name + written, class_suffix, remaining);
> 
> strncpy has other problems though, not zero-terminating on truncation,
> and weirdly enough \0 padding destination to its end (none of which
> apply here).
> 
> I usually rather use snprintf, e.g.:
> 
> jio_snprintf(file_name, bufferlen, "%s%s", class_name, class_suffix);
> 
> it is shorter and one needs not to know the size of the components and
> it takes care of truncation correctly and does not waste time to pad
> the destination.
> 
> Just my 5c...
> 
> Cheers, Thomas
> 
> 
> 
> 
>> Thanks,
>> David
>>
>> On 7/12/2018 6:44 am, Harold David Seigel wrote:
>>> Hi Dmitry,
>>>
>>> Could you change the type of class_suffix_len from int to size_t ?
>>> Otherwise, it looks good.  (I don't need to see a new webrev.)
>>>
>>> Thanks, Harold
>>>
>>>
>>> On 12/6/2018 3:19 PM, Dmitry Chuyko wrote:
>>>> Hello,
>>>>
>>>> Please review a small fix for an error reported by GCC 8. It's a false
>>>> positive and minor code clean-up silences the compiler: suffix length
>>>> is extracted into a variable.
>>>>
>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8214707
>>>> webrev: http://cr.openjdk.java.net/~dchuyko/8214707/webrev.00/
>>>> testing: submit repo
>>>> (mach5-one-dchuyko-JDK-8214707-20181206-1906-13407: PASSED)
>>>>
>>>> -Dmitry
>>>>
>>>