On Sat, 20 Dec 2025 02:39:30 GMT, Chris Plummer <[email protected]> wrote:
>> Alex Menkov has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> feedback
>
> src/jdk.jdwp.agent/share/native/libjdwp/eventFilter.c line 980:
>
>> 978: {
>> 979: // PlatformThreadsFilter contains nothing useful
>> 980: //PlatformThreadsFilter *filter = &FILTER(node,
>> index).u.PlatformThreadsOnly;
>
> I think you can just delete this.
deleted
> src/jdk.jdwp.agent/share/native/libjdwp/threadControl.c line 2678:
>
>> 2676: if (error != JVMTI_ERROR_NONE) {
>> 2677: EXIT_ERROR(error, "cannot get current thread");
>> 2678: }
>
> I'm not sure about his one since it is a behavior change. For example, we
> have:
>
> jthread thread = threadControl_currentThread();
> if ((thread != NULL) && (!threadControl_isDebugThread(thread))) {
> threadControl_setPendingInterrupt(thread);
> }
Looks like it can be called in wrong phase. dropped EXIT_ERROR and added
explicit note that the function returns NULL on error
> src/jdk.jdwp.agent/share/native/libjdwp/util.h line 64:
>
>> 62: /* To handle "format string is not a string literal" warning. */
>> 63: #if !defined(_MSC_VER)
>> 64: #define ATTRIBUTE_PRINTF(fmt, vargs) __attribute__((format(printf,
>> fmt, vargs)))
>
> Why not ATTRIBUTE_FORMAT?
printf is essential here, attribute(format) also support other functions -
scanf, strftime, strfmon
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28937#discussion_r2641151638
PR Review Comment: https://git.openjdk.org/jdk/pull/28937#discussion_r2641153882
PR Review Comment: https://git.openjdk.org/jdk/pull/28937#discussion_r2641157278
