remove useless comments
The comments considering checking share/ directory was there for
almost 13 years, yet nobody ever trying to add the checking, and
there seems never any trouble for not checking it, then I think
we should remove those comments.
diff --git a/src/backend/postmaster/postmaster.c
b/src/backend/postmaster/postmaster.c
index 81cb585891..ecdc59ce5e 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -1581,11 +1581,6 @@ getInstallationPaths(const char *argv0)
errhint("This may indicate an
incomplete PostgreSQL installation, or that the file \"%s\" has been
moved away from its proper location.",
my_exec_path)));
FreeDir(pdir);
-
- /*
- * XXX is it worth similarly checking the share/ directory? If the lib/
- * directory is there, then share/ probably is too.
- */
}--
Regards
Junwang Zhao
Attachments:
0001-comments-remove-useless-comments.patchapplication/octet-stream; name=0001-comments-remove-useless-comments.patchDownload
From 0ad6227c25a7080de6ce1157b9b19703747f101b Mon Sep 17 00:00:00 2001
From: Junwang Zhao <zhjwpku@gmail.com>
Date: Tue, 9 Aug 2022 23:33:55 +0800
Subject: [PATCH v1] [comments] remove useless comments
The comments considering checking share/ directory was there for
almost 13 years, yet nobody ever trying to add the checking, and
there seems never any trouble for not checking it, then I think
we should remove those comments.
Signed-off-by: Junwang Zhao <zhjwpku@gmail.com>
---
src/backend/postmaster/postmaster.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 81cb585891..ecdc59ce5e 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -1581,11 +1581,6 @@ getInstallationPaths(const char *argv0)
errhint("This may indicate an incomplete PostgreSQL installation, or that the file \"%s\" has been moved away from its proper location.",
my_exec_path)));
FreeDir(pdir);
-
- /*
- * XXX is it worth similarly checking the share/ directory? If the lib/
- * directory is there, then share/ probably is too.
- */
}
/*
--
2.33.0
Junwang Zhao <zhjwpku@gmail.com> writes:
The comments considering checking share/ directory was there for
almost 13 years, yet nobody ever trying to add the checking, and
there seems never any trouble for not checking it, then I think
we should remove those comments.
I think that comment is valuable. It shows that checking the
sibling directories was considered and didn't seem worthwhile.
Perhaps it should be rephrased in a more positive way (without XXX),
but merely deleting it is a net negative because future hackers
would have to reconstruct that reasoning.
BTW, we're working in a 30+-year-old code base, so the mere fact
that a comment has been there a long time does not make it bad.
regards, tom lane
Fair enough, the rephased version of the comments is in the attachment,
please take a look.
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -1583,8 +1583,8 @@ getInstallationPaths(const char *argv0)
FreeDir(pdir); /*
- * XXX is it worth similarly checking the share/ directory? If the lib/
- * directory is there, then share/ probably is too.
+ * It's not worth checking the share/ directory. If the lib/ directory
+ * is there, then share/ probably is too.
*/
}On Tue, Aug 9, 2022 at 11:50 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Junwang Zhao <zhjwpku@gmail.com> writes:
The comments considering checking share/ directory was there for
almost 13 years, yet nobody ever trying to add the checking, and
there seems never any trouble for not checking it, then I think
we should remove those comments.I think that comment is valuable. It shows that checking the
sibling directories was considered and didn't seem worthwhile.
Perhaps it should be rephrased in a more positive way (without XXX),
but merely deleting it is a net negative because future hackers
would have to reconstruct that reasoning.BTW, we're working in a 30+-year-old code base, so the mere fact
that a comment has been there a long time does not make it bad.regards, tom lane
--
Regards
Junwang Zhao
Attachments:
0001-comments-rephrase-the-comment-in-a-more-positive-way.patchapplication/octet-stream; name=0001-comments-rephrase-the-comment-in-a-more-positive-way.patchDownload
From 17332a87c324d6f1cf976b7d3dc114cb5668882e Mon Sep 17 00:00:00 2001
From: Junwang Zhao <zhjwpku@gmail.com>
Date: Tue, 9 Aug 2022 23:33:55 +0800
Subject: [PATCH v1] [comments] rephrase the comment in a more positive way
rephrase the comment in a more positive way so that future
hackers will not try to remove this valuable information.
Signed-off-by: Junwang Zhao <zhjwpku@gmail.com>
---
src/backend/postmaster/postmaster.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 81cb585891..14ab3732f6 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -1583,8 +1583,8 @@ getInstallationPaths(const char *argv0)
FreeDir(pdir);
/*
- * XXX is it worth similarly checking the share/ directory? If the lib/
- * directory is there, then share/ probably is too.
+ * It's not worth checking the share/ directory. If the lib/ directory
+ * is there, then share/ probably is too.
*/
}
--
2.33.0
On Wed, Aug 10, 2022 at 12:24:02AM +0800, Junwang Zhao wrote:
Fair enough, the rephased version of the comments is in the attachment,
please take a look.--- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -1583,8 +1583,8 @@ getInstallationPaths(const char *argv0) FreeDir(pdir);/* - * XXX is it worth similarly checking the share/ directory? If the lib/ - * directory is there, then share/ probably is too. + * It's not worth checking the share/ directory. If the lib/ directory + * is there, then share/ probably is too. */ }
Patch applied to master.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.