On Wed, 25 May 2022 09:16:43 GMT, Yasumasa Suenaga <ysuen...@openjdk.org> wrote:

>> I saw some compiler warnings when I tried to build OpenJDK with GCC 12.0.1 
>> on Fedora 36.
>> As you can see, the warnings spreads several areas. Let me know if I should 
>> separate them by area.
>> 
>> * -Wstringop-overflow
>>     * src/hotspot/share/oops/array.hpp
>>     * 
>> src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.inline.hpp
>> 
>> In member function 'void Array<T>::at_put(int, const T&) [with T = unsigned 
>> char]',
>>     inlined from 'void ConstantPool::tag_at_put(int, jbyte)' at 
>> /home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/constantPool.hpp:126:64,
>>     inlined from 'void ConstantPool::method_at_put(int, int, int)' at 
>> /home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/constantPool.hpp:380:15,
>>     inlined from 'ConstantPool* 
>> BytecodeConstantPool::create_constant_pool(JavaThread*) const' at 
>> /home/ysuenaga/github-forked/jdk/src/hotspot/share/classfile/bytecodeAssembler.cpp:85:26:
>
> Yasumasa Suenaga has updated the pull request incrementally with two 
> additional commits since the last revision:
> 
>  - Change Array::data() implementation
>  - Avoid stringop-overflow warning in jfrTraceIdBits.inline.hpp

Mostly good, but I missed a problem with an earlier part of the change.  Sorry 
I didn't notice sooner.

src/java.base/unix/native/libjli/java_md_common.c line 133:

> 131: 
> 132:     snprintf_result = JLI_Snprintf(name, sizeof(name), "%s%c%s", indir, 
> FILE_SEPARATOR, cmd);
> 133:     if ((snprintf_result < 0) && (snprintf_result >= (int)sizeof(name))) 
> {

That should be `||` rather than `&&`.

src/java.base/unix/native/libjli/java_md_common.c line 135:

> 133:     if ((snprintf_result < 0) && (snprintf_result >= (int)sizeof(name))) 
> {
> 134:       return 0;
> 135:     }

Pre-existing: It seems odd that this returns `0` above and below, rather than 
returning `NULL`.  I think there are one or two other places in this file that 
are similarly using literal `0` for a null pointer, though others are using 
`NULL`.  Something to report and clean up separately from this change.

-------------

Changes requested by kbarrett (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/8646

Reply via email to