On Tue, 19 May 2026 02:02:54 GMT, SendaoYan <[email protected]> wrote:

>> Hi all,
>> 
>> Function handleMessage in file 
>> src/jdk.sctp/unix/native/libsctp/SctpChannelImpl.c declare 'struct 
>> controlData cdata[1]' but do not initiali the struct variable. After the 
>> declation, the cdata will write the related data through funciton call 
>> 'getControlData(msg, cdata)', and the data will be readed through 
>> '(*env)->NewObject'. During 'getControlData(msg, cdata)' call the cdate 
>> maybe unchange and then return.
>> 
>> In C/C++, read the variable which has not been uninitializaed is undefined 
>> behavior. I think it's better to initial the cdata, this will avoid the 
>> compiler use too aggressive optimilization.
>> 
>> Before this PR, com/sun/nio/sctp/SctpChannel/ReceiveIntoDirect.java crash 'C 
>>  [libsctp.so+0x3b3e]  handleMessage+0x4e' with clang23/llvm23 release build 
>> . After this PR test run passed.
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> SendaoYan has updated the pull request with a new target base due to a merge 
> or a rebase. The incremental webrev excludes the unrelated changes brought in 
> by the merge/rebase. The pull request contains two additional commits since 
> the last revision:
> 
>  - Merge branch 'openjdk:master' into jbs8381851
>  - 8381851: handleMessage use uninitialized struct

Marked as reviewed by jpai (Reviewer).

src/jdk.sctp/unix/native/libsctp/SctpChannelImpl.c line 392:

> 390:    jboolean isEOR, struct sockaddr* sap) {
> 391:     jobject isa, resultObj;
> 392:     struct controlData cdata[1] = {0};

Initializing each of the fields on the `controlData` struct instance to `0`, 
looks OK to me.

A larger question however is how this code should deal with the case where the 
implementation in `getControlData(msg, cdata)` doesn't set/overwrite this 
instance's field values i.e. if there are no messages which have 
`cmsg->cmsg_level == IPPROTO_SCTP && cmsg->cmsg_type == SCTP_SNDRCV`. It looks 
like we don't check for that possibility after we return from that call to 
`getControlData()` and go ahead and construct the MessageInfoImpl with the now 
default values of `0` for these fields. But that part of the code is 
pre-existing and your proposed change here in fact improves the situation by 
setting it to a default value.

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

PR Review: https://git.openjdk.org/jdk/pull/30664#pullrequestreview-4327767993
PR Review Comment: https://git.openjdk.org/jdk/pull/30664#discussion_r3273344205

Reply via email to