public inbox for [email protected]
help / color / mirror / Atom feedFrom: Bertrand Drouvot <[email protected]>
To: PostgreSQL Hackers <[email protected]>
Subject: Avoid circular header file dependency
Date: Sat, 26 Apr 2025 03:20:09 +0000
Message-ID: <[email protected]> (raw)
Hi hackers,
While working on wait events I faced some compilation issues due to circular
header file dependency (introduced in fa88928470b5) between wait_event.h and
wait_event_types.h. Those files have include guards but could still lead to
compilation issues in some cases due to the circular dependency.
Currently, on HEAD, this doesn't cause any issues but I think it's better to
avoid circular header file dependencies (harder to maintain and understand).
Please find attached a patch to $SUBJECT between those 2 header files: it
extracts (in a new header file) from wait_event.h what is strictly needed in
wait_event_types.h.
Out of curiosity, I ran clang-tidy with misc-header-include-cycle ([1]) and it
also reports:
../src/pl/plpython/plpy_util.h:9:10: warning: circular header file dependency detected while including 'plpython.h'
This one worries me less because plpy_util.h only contains simple external
function declarations.
I was surprised that clang-tidy does report only the plpy_util.h one (in addition
to the wait_event_types.h one) so I wondered if misc-header-include-cycle was
failing to discover circular dependencies in nested cases.
I did a quick test with: a.h → c.h → b.h → d.h → a.h and it found it:
"
../d.h:6:10: warning: circular header file dependency detected while including 'a.h'
6 | #include "a.h"
| ^
../b.h:6:10: note: 'd.h' included from here
6 | #include "d.h"
| ^
../c.h:6:10: note: 'b.h' included from here
6 | #include "b.h"
| ^
../a.h:6:10: note: 'c.h' included from here
6 | #include "c.h"
| ^
../main.c:2:10: note: 'a.h' included from here
2 | #include "a.h"
"
So it looks like that our code base only contains those 2: plpy_util.h and
wait_event_types.h cases.
Thoughts?
[1]: https://clang.llvm.org/extra/clang-tidy/checks/misc/header-include-cycle.html
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
[text/x-diff] v1-0001-Avoid-including-wait_event.h-in-wait_event_types..patch (3.1K, ../[email protected]/2-v1-0001-Avoid-including-wait_event.h-in-wait_event_types..patch)
download | inline diff:
From e001863ab8645b5b6c2021c857ac541e230398dd Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email protected]>
Date: Thu, 24 Apr 2025 17:11:03 +0000
Subject: [PATCH v1] Avoid including wait_event.h in wait_event_types.h
Adding wait_class_constants.h to avoid including wait_event.h in
wait_event_types.h (that produced a circular include).
---
.../activity/generate-wait_event_types.pl | 2 +-
src/include/utils/wait_class_constants.h | 29 +++++++++++++++++++
src/include/utils/wait_event.h | 17 ++---------
3 files changed, 32 insertions(+), 16 deletions(-)
create mode 100644 src/include/utils/wait_class_constants.h
diff --git a/src/backend/utils/activity/generate-wait_event_types.pl b/src/backend/utils/activity/generate-wait_event_types.pl
index 171bf2ae632..52b6b7b2ae6 100644
--- a/src/backend/utils/activity/generate-wait_event_types.pl
+++ b/src/backend/utils/activity/generate-wait_event_types.pl
@@ -168,7 +168,7 @@ if ($gen_code)
printf $h $header_comment, 'wait_event_types.h';
printf $h "#ifndef WAIT_EVENT_TYPES_H\n";
printf $h "#define WAIT_EVENT_TYPES_H\n\n";
- printf $h "#include \"utils/wait_event.h\"\n\n";
+ printf $h "#include \"utils/wait_class_constants.h\"\n\n";
printf $c $header_comment, 'pgstat_wait_event.c';
diff --git a/src/include/utils/wait_class_constants.h b/src/include/utils/wait_class_constants.h
new file mode 100644
index 00000000000..286f9c2daec
--- /dev/null
+++ b/src/include/utils/wait_class_constants.h
@@ -0,0 +1,29 @@
+/*-------------------------------------------------------------------------
+ * wait_class_constants.h
+ * Constants related to wait classes
+ *
+ * Copyright (c) 2001-2025, PostgreSQL Global Development Group
+ *
+ * src/include/utils/wait_class_constants.h
+ * ----------
+ */
+#ifndef WAIT_CLASS_CONSTANTS_H
+#define WAIT_CLASS_CONSTANTS_H
+
+
+/* ----------
+ * Wait Classes
+ * ----------
+ */
+#define PG_WAIT_LWLOCK 0x01000000U
+#define PG_WAIT_LOCK 0x03000000U
+#define PG_WAIT_BUFFERPIN 0x04000000U
+#define PG_WAIT_ACTIVITY 0x05000000U
+#define PG_WAIT_CLIENT 0x06000000U
+#define PG_WAIT_EXTENSION 0x07000000U
+#define PG_WAIT_IPC 0x08000000U
+#define PG_WAIT_TIMEOUT 0x09000000U
+#define PG_WAIT_IO 0x0A000000U
+#define PG_WAIT_INJECTIONPOINT 0x0B000000U
+
+#endif /* WAIT_CLASS_CONSTANTS_H */
diff --git a/src/include/utils/wait_event.h b/src/include/utils/wait_event.h
index b8cb3e5a430..c3c1ce6ef2e 100644
--- a/src/include/utils/wait_event.h
+++ b/src/include/utils/wait_event.h
@@ -10,21 +10,8 @@
#ifndef WAIT_EVENT_H
#define WAIT_EVENT_H
-
-/* ----------
- * Wait Classes
- * ----------
- */
-#define PG_WAIT_LWLOCK 0x01000000U
-#define PG_WAIT_LOCK 0x03000000U
-#define PG_WAIT_BUFFERPIN 0x04000000U
-#define PG_WAIT_ACTIVITY 0x05000000U
-#define PG_WAIT_CLIENT 0x06000000U
-#define PG_WAIT_EXTENSION 0x07000000U
-#define PG_WAIT_IPC 0x08000000U
-#define PG_WAIT_TIMEOUT 0x09000000U
-#define PG_WAIT_IO 0x0A000000U
-#define PG_WAIT_INJECTIONPOINT 0x0B000000U
+/* wait class constants */
+#include "utils/wait_class_constants.h"
/* enums for wait events */
#include "utils/wait_event_types.h"
--
2.34.1
view thread (5+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: Avoid circular header file dependency
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox