seemingly useless #include recently added
Hello.
While working on a patch, I noticed that a rcent commit (d4e71df6d75)
added an apparently unnecessary inclusion of guc.h in smgr.h.
The only change made by the commit to the file is the added #include
directive, which doesn't seem to be functioning, and the build
actually suceeds without it. Moreover, it brings in some
server-related stuff when I incluce smgr.h in storage_xlog.h, causing
compilation issues for pg_rewind.
Should we remove it? Please find the attached patch.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Attachments:
0001-Remove-unnecessary-include.patchtext/x-patch; charset=us-asciiDownload
From 75d35c25d4cfaeb6fb40e2d451381beac1d5475c Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Tue, 25 Apr 2023 11:43:32 +0900
Subject: [PATCH] Remove unnecessary include
The recently added inclusion of guc.h in msgr.h is not necessary and
introduces more server-related stuff. Removing the directive helps
avoid potential issues with including sgmr.h in frontends.
---
src/include/storage/smgr.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/include/storage/smgr.h b/src/include/storage/smgr.h
index 17fba6f91a..a9a179aaba 100644
--- a/src/include/storage/smgr.h
+++ b/src/include/storage/smgr.h
@@ -17,7 +17,6 @@
#include "lib/ilist.h"
#include "storage/block.h"
#include "storage/relfilelocator.h"
-#include "utils/guc.h"
/*
* smgr.c maintains a table of SMgrRelation objects, which are essentially
--
2.31.1
Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:
While working on a patch, I noticed that a rcent commit (d4e71df6d75)
added an apparently unnecessary inclusion of guc.h in smgr.h.
Yes, that seems quite awful, and I also wonder why it changed fd.h.
Adding #include's to header files is generally not the first choice.
regards, tom lane
On Tue, Apr 25, 2023 at 3:12 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:
While working on a patch, I noticed that a rcent commit (d4e71df6d75)
added an apparently unnecessary inclusion of guc.h in smgr.h.Yes, that seems quite awful, and I also wonder why it changed fd.h.
Adding #include's to header files is generally not the first choice.
Agreed for smgr.h. Will push when I'm back at a real computer soon,
or +1 from me if someone else wants to. It must have been left over
from an earlier version that had a different arrangement with multiple
GUCs in different places and might have needed GUC-related types to
declare the check functions or something like that; sorry. As for
fd.h, the reason it now includes <fcntl.h> is that fd.h tests whether
O_DIRECT is defined, so in fact that was an omission from 2dbe8905
which moved the #if defined(O_DIRECT) stuff from xlogdefs.h to fd.h
but failed to move the #include with it; I will check if something
needs to be back-patched there.