unnecessary #include "pg_getopt.h"?
Hi,
While working on [1]/messages/by-id/d660ef741ce3d82f3b4283f1cafd576c@oss.nttdata.com -- Regards,, I thought there seems to be unnecessary #include
"pg_getopt.h".
getopt_long.h has already included pg_getopt.h, but some files include
both getopt.h and getopt_long.h.
[1]: /messages/by-id/d660ef741ce3d82f3b4283f1cafd576c@oss.nttdata.com -- Regards,
/messages/by-id/d660ef741ce3d82f3b4283f1cafd576c@oss.nttdata.com
--
Regards,
--
Atsushi Torikoshi
NTT DATA CORPORATION
Attachments:
v1-0001-Remove-unnecessary-inlucde-pg_getopt.patchtext/x-diff; name=v1-0001-Remove-unnecessary-inlucde-pg_getopt.patchDownload
From 0c11610505a3f25d8d65e9f8969328bba89f5c44 Mon Sep 17 00:00:00 2001
From: Atsushi Torikoshi <torikoshia@oss.nttdata.com>
Date: Mon, 22 May 2023 17:37:25 +0900
Subject: [PATCH v1] Remove unnecessary include pg_getopt.h
---
contrib/oid2name/oid2name.c | 1 -
contrib/vacuumlo/vacuumlo.c | 1 -
src/bin/pg_checksums/pg_checksums.c | 1 -
src/bin/pg_controldata/pg_controldata.c | 1 -
src/bin/pg_resetwal/pg_resetwal.c | 1 -
5 files changed, 5 deletions(-)
diff --git a/contrib/oid2name/oid2name.c b/contrib/oid2name/oid2name.c
index e8c1e2c97b..32b2b43608 100644
--- a/contrib/oid2name/oid2name.c
+++ b/contrib/oid2name/oid2name.c
@@ -15,7 +15,6 @@
#include "common/string.h"
#include "getopt_long.h"
#include "libpq-fe.h"
-#include "pg_getopt.h"
/* an extensible array to keep track of elements to show */
typedef struct
diff --git a/contrib/vacuumlo/vacuumlo.c b/contrib/vacuumlo/vacuumlo.c
index 8941262731..852486dc11 100644
--- a/contrib/vacuumlo/vacuumlo.c
+++ b/contrib/vacuumlo/vacuumlo.c
@@ -27,7 +27,6 @@
#include "common/string.h"
#include "getopt_long.h"
#include "libpq-fe.h"
-#include "pg_getopt.h"
#define BUFSIZE 1024
diff --git a/src/bin/pg_checksums/pg_checksums.c b/src/bin/pg_checksums/pg_checksums.c
index 19eb67e485..03678851ef 100644
--- a/src/bin/pg_checksums/pg_checksums.c
+++ b/src/bin/pg_checksums/pg_checksums.c
@@ -27,7 +27,6 @@
#include "common/logging.h"
#include "fe_utils/option_utils.h"
#include "getopt_long.h"
-#include "pg_getopt.h"
#include "storage/bufpage.h"
#include "storage/checksum.h"
#include "storage/checksum_impl.h"
diff --git a/src/bin/pg_controldata/pg_controldata.c b/src/bin/pg_controldata/pg_controldata.c
index c390ec51ce..52d7c170e1 100644
--- a/src/bin/pg_controldata/pg_controldata.c
+++ b/src/bin/pg_controldata/pg_controldata.c
@@ -27,7 +27,6 @@
#include "common/controldata_utils.h"
#include "common/logging.h"
#include "getopt_long.h"
-#include "pg_getopt.h"
static void
usage(const char *progname)
diff --git a/src/bin/pg_resetwal/pg_resetwal.c b/src/bin/pg_resetwal/pg_resetwal.c
index e7ef2b8bd0..58ad79d38f 100644
--- a/src/bin/pg_resetwal/pg_resetwal.c
+++ b/src/bin/pg_resetwal/pg_resetwal.c
@@ -56,7 +56,6 @@
#include "common/restricted_token.h"
#include "common/string.h"
#include "getopt_long.h"
-#include "pg_getopt.h"
#include "storage/large_object.h"
static ControlFileData ControlFile; /* pg_control values */
--
2.39.2
On Mon, May 22, 2023 at 06:48:37PM +0900, torikoshia wrote:
While working on [1], I thought there seems to be unnecessary #include
"pg_getopt.h".
getopt_long.h has already included pg_getopt.h, but some files include both
getopt.h and getopt_long.h.
Right, these could be removed. I am not seeing other places in the
tree that include both. That's always nice to clean up.
--
Michael
Hi,
On 2023-05-24 09:59:18 +0900, Michael Paquier wrote:
On Mon, May 22, 2023 at 06:48:37PM +0900, torikoshia wrote:
While working on [1], I thought there seems to be unnecessary #include
"pg_getopt.h".
getopt_long.h has already included pg_getopt.h, but some files include both
getopt.h and getopt_long.h.Right, these could be removed. I am not seeing other places in the
tree that include both. That's always nice to clean up.
This feels more like a matter of taste to me than anything. At least some of
the files touched in the patch use optarg, opterr etc. - which are declared in
pg_getopt.h. Making it reasonable to directly include pg_getopt.h.
I don't really see a need to change anything here?
Greetings,
Andres Freund
On Tue, May 23, 2023 at 06:37:59PM -0700, Andres Freund wrote:
This feels more like a matter of taste to me than anything.
Yup, it is.
At least some of
the files touched in the patch use optarg, opterr etc. - which are declared in
pg_getopt.h. Making it reasonable to directly include pg_getopt.h.
getopt_long.h is included in 21 places of src/bin/, with all of them
touching optarg, while only four of them include pg_getopt.h. So
removing them as suggested makes sense. I agree with the tasting
matter as well, still there is also a consistency matter.
--
Michael