RFR: 8324668: JDWP process management needs more efficient file descriptor handling [v3]

Jaikiran Pai jpai at openjdk.org
Mon Jan 29 10:05:28 UTC 2024


On Mon, 29 Jan 2024 04:36:35 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Jaikiran Pai has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - log a warning message if we fallback to slower logic of closing file descriptors
>>  - ignore return values, cast to void
>
> src/jdk.jdwp.agent/unix/native/libjdwp/exec_md.c line 52:
> 
>> 50: 
>> 51: int
>> 52: isAsciiDigit(char c)
> 
> The standard `isdigit` function from `ctype.h` does this.

Done.

> src/jdk.jdwp.agent/unix/native/libjdwp/exec_md.c line 75:
> 
>> 73: // input/output/error file descriptors will not be closed
>> 74: // by this function. this function returns 0 on failure
>> 75: // and 1 on success
> 
> Pre-existing nit: Please start sentences with a capital and end with a period. Thanks. This applies through the copied code.

I've fixed the code comments to follow this suggestion. I've left single sentence comments as is. If there's a preference to change them too, do let me know and I'll update those too.

> src/jdk.jdwp.agent/unix/native/libjdwp/exec_md.c line 103:
> 
>> 101: #endif
>> 102: 
>> 103:     if ((dp = opendir(FD_DIR)) == NULL)
> 
> It is better to log the failure here, where you can actually report the reason.

Done.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17588#discussion_r1469351351
PR Review Comment: https://git.openjdk.org/jdk/pull/17588#discussion_r1469351192
PR Review Comment: https://git.openjdk.org/jdk/pull/17588#discussion_r1469349326


More information about the serviceability-dev mailing list