On Sat, 27 Jan 2024 01:18:09 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> src/jdk.jdwp.agent/unix/native/libjdwp/exec_md.c line 129:
>>
>>> 127: * and assume all were opened for the parent process and
>>> 128: * copied over to this child process. we close them all */
>>> 129: const rlim_t max_fd = sysconf(_SC_OPEN_MAX);
>>
>> Why not use `int`, like the original code, instead of `rlim_t` - as per man
>> page `close()` expects `int`, not `rlim_t`, ex:
>>
>>
>> const int max_fd = (int)sysconf(_SC_OPEN_MAX);
>> JDI_ASSERT(max_fd != -1);
>> int fd;
>> /* leave out standard input/output/error file descriptors */
>> for (fd = STDERR_FILENO + 1; fd < max_fd; fd++) {
>> (void)close(fd);
>> }
>
> Hello Gerard, my understanding is that the limit value configured may exceed
> the int range. I wanted to avoid the overflow by casting it to int in such
> cases. I had noticed close() takes an int, but I couldn't think of any other
> way of avoiding the overflow at this place.
>
> In the JVM parent process we do however limit it to INT_MAX. So maybe I
> should assume that it will be an int cast it to int here, like you suggest,
> and add a code comment explaining this? Does that sound OK?
That is fine, but in that case we should also do:
`JDI_ASSERT(max_fd <= INT_MAX);`
in case `sysconf(_SC_OPEN_MAX)` returns value greater than `INT_MAX`, since
`close()` only accepts `int`.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17588#discussion_r1469976093
