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

[David Holmes]

On Wed, 31 Jan 2024 07:52:18 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> src/jdk.jdwp.agent/unix/native/libjdwp/exec_md.c line 35:
>> 
>>> 33: #include "sys.h"
>>> 34: #include "util.h"
>>> 35: #include "error_messages.h"
>> 
>> Nit: to maintain include sort order this should have gone where `log_messages.h` was removed.
>
> Hello David, I had actually first put it in that line you noted, but that then lead to a compilation error:
> 
> 
> In file included from /jdk/src/jdk.jdwp.agent/unix/native/libjdwp/exec_md.c:33:
> /jdk/src/jdk.jdwp.agent/share/native/libjdwp/error_messages.h:35:20: error: unknown type name 'FILE'
> void print_message(FILE *fp, const char *prefix,  const char *suffix,
>                    ^
> /jdk/src/jdk.jdwp.agent/share/native/libjdwp/error_messages.h:41:29: error: a parameter list without types is only allowed in a function definition
> const char * jvmtiErrorText(jvmtiError);
>                             ^
> /jdk/src/jdk.jdwp.agent/share/native/libjdwp/error_messages.h:43:28: error: a parameter list without types is only allowed in a function definition
> const char * jdwpErrorText(jdwpError);
>                            ^
> 3 errors generated.
> 
> 
> 
> I think the reason for those compilation errors is that `error_messages.h` and a few other header files are not explicitly listing their necessary includes and are relying on transitive includes. So I decided to take the easy route out, by adding the `error_messages.h` after the `util.h` to rely on the transitive includes to get past that compilation error. 
> 
> I think to properly fix that compile error, this following change (which I tested locally and works) would be needed:
> 
> 
> diff --git a/src/jdk.jdwp.agent/share/native/libjdwp/JDWP.h b/src/jdk.jdwp.agent/share/native/libjdwp/JDWP.h
> index 92a9f457e8a..2683c6791a1 100644
> --- a/src/jdk.jdwp.agent/share/native/libjdwp/JDWP.h
> +++ b/src/jdk.jdwp.agent/share/native/libjdwp/JDWP.h
> @@ -26,6 +26,8 @@
>  #ifndef JDWP_JDWP_H
>  #define JDWP_JDWP_H
>  
> +#include "jni_md.h"
> +#include "jvmti.h"
>  #include "JDWPCommands.h"
>  
>  /*
> diff --git a/src/jdk.jdwp.agent/share/native/libjdwp/error_messages.h b/src/jdk.jdwp.agent/share/native/libjdwp/error_messages.h
> index 4126b76f226..810ab384f1a 100644
> --- a/src/jdk.jdwp.agent/share/native/libjdwp/error_messages.h
> +++ b/src/jdk.jdwp.agent/share/native/libjdwp/error_messages.h
> @@ -26,6 +26,9 @@
>  #ifndef JDWP_ERROR_MESSAGES_H
>  #define JDWP_ERROR_MESSAGES_H
>  
> +#include <stdio.h>
> +#include "JDWP.h"
> +
>  /* It is assumed that ALL strings are UTF-8 safe on entry */
>  #define TTY_MESSAGE(args) ( tty_message args )
>  #define ERROR_MESSAGE(args) ( \
> diff --git a/src/jdk.jdwp.agent/unix/native/libjdwp/exec_md.c b/src/jdk.jdwp.agent/unix/native/libjdwp/exec_md.c
> index b43d4de80fc..cf0d00db192 100644
> --- a/src/jdk.jdwp.agent/unix/native/libjdwp/exec_md.c
> +++ b/src/jdk.jdwp.agent...

Please file a JBS issue, but no need for you to address it. Thanks.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17588#discussion_r1473821677