On Wed, 31 Jan 2024 07:52:18 GMT, Jaikiran Pai <j...@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
