pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedDead code in adminpack
4+ messages / 3 participants
[nested] [flat]
Commits from this thread:
2020-02-14 11f063b0a9 Remove some dead code in contrib/adminpack/
* Dead code in adminpack
@ 2020-02-13 11:15 Antonin Houska <ah@cybertec.at>
0 siblings, 2 replies; 4+ messages in thread
From: Antonin Houska @ 2020-02-13 11:15 UTC (permalink / raw)
To: pgsql-hackers
I've noticed that convert_and_check_filename() is always passed false for the
"logAllowed" argument - someone probably forgot to remove the argument when it
was decided that log files are no longer accepted. If the argument was removed,
the function would become a bit simpler, see the patch.
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
Attachments:
adminpack_dead_code.patch (text/x-diff, 2.6K) download
diff --git a/contrib/adminpack/adminpack.c b/contrib/adminpack/adminpack.c
index 29b46aea3e..f33cc6e532 100644
--- a/contrib/adminpack/adminpack.c
+++ b/contrib/adminpack/adminpack.c
@@ -69,10 +69,10 @@ typedef struct
* Convert a "text" filename argument to C string, and check it's allowable.
*
* Filename may be absolute or relative to the DataDir, but we only allow
- * absolute paths that match DataDir or Log_directory.
+ * absolute paths that match DataDir.
*/
static char *
-convert_and_check_filename(text *arg, bool logAllowed)
+convert_and_check_filename(text *arg)
{
char *filename = text_to_cstring(arg);
@@ -99,9 +99,7 @@ convert_and_check_filename(text *arg, bool logAllowed)
* Allow absolute paths if within DataDir or Log_directory, even
* though Log_directory might be outside DataDir.
*/
- if (!path_is_prefix_of_path(DataDir, filename) &&
- (!logAllowed || !is_absolute_path(Log_directory) ||
- !path_is_prefix_of_path(Log_directory, filename)))
+ if (!path_is_prefix_of_path(DataDir, filename))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("absolute path not allowed")));
@@ -185,7 +183,7 @@ pg_file_write_internal(text *file, text *data, bool replace)
char *filename;
int64 count = 0;
- filename = convert_and_check_filename(file, false);
+ filename = convert_and_check_filename(file);
if (!replace)
{
@@ -228,7 +226,7 @@ pg_file_sync(PG_FUNCTION_ARGS)
char *filename;
struct stat fst;
- filename = convert_and_check_filename(PG_GETARG_TEXT_PP(0), false);
+ filename = convert_and_check_filename(PG_GETARG_TEXT_PP(0));
if (stat(filename, &fst) < 0)
ereport(ERROR,
@@ -319,13 +317,13 @@ pg_file_rename_internal(text *file1, text *file2, text *file3)
*fn3;
int rc;
- fn1 = convert_and_check_filename(file1, false);
- fn2 = convert_and_check_filename(file2, false);
+ fn1 = convert_and_check_filename(file1);
+ fn2 = convert_and_check_filename(file2);
if (file3 == NULL)
fn3 = NULL;
else
- fn3 = convert_and_check_filename(file3, false);
+ fn3 = convert_and_check_filename(file3);
if (access(fn1, W_OK) < 0)
{
@@ -411,7 +409,7 @@ pg_file_unlink(PG_FUNCTION_ARGS)
requireSuperuser();
- filename = convert_and_check_filename(PG_GETARG_TEXT_PP(0), false);
+ filename = convert_and_check_filename(PG_GETARG_TEXT_PP(0));
if (access(filename, W_OK) < 0)
{
@@ -449,7 +447,7 @@ pg_file_unlink_v1_1(PG_FUNCTION_ARGS)
{
char *filename;
- filename = convert_and_check_filename(PG_GETARG_TEXT_PP(0), false);
+ filename = convert_and_check_filename(PG_GETARG_TEXT_PP(0));
if (access(filename, W_OK) < 0)
{
* Re: Dead code in adminpack
@ 2020-02-13 11:41 Julien Rouhaud <rjuju123@gmail.com>
parent: Antonin Houska <ah@cybertec.at>
1 sibling, 1 reply; 4+ messages in thread
From: Julien Rouhaud @ 2020-02-13 11:41 UTC (permalink / raw)
To: Antonin Houska <ah@cybertec.at>; +Cc: pgsql-hackers
On Thu, Feb 13, 2020 at 12:14 PM Antonin Houska <ah@cybertec.at> wrote:
>
> I've noticed that convert_and_check_filename() is always passed false for the
> "logAllowed" argument - someone probably forgot to remove the argument when it
> was decided that log files are no longer accepted. If the argument was removed,
> the function would become a bit simpler, see the patch.
Indeed, and actually I don't see when this codepath was reachable.
Patch LGTM.
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Dead code in adminpack
@ 2020-02-13 11:45 Michael Paquier <michael@paquier.xyz>
parent: Antonin Houska <ah@cybertec.at>
1 sibling, 0 replies; 4+ messages in thread
From: Michael Paquier @ 2020-02-13 11:45 UTC (permalink / raw)
To: Antonin Houska <ah@cybertec.at>; +Cc: pgsql-hackers
On Thu, Feb 13, 2020 at 12:15:39PM +0100, Antonin Houska wrote:
> I've noticed that convert_and_check_filename() is always passed false for the
> "logAllowed" argument - someone probably forgot to remove the argument when it
> was decided that log files are no longer accepted. If the argument was removed,
> the function would become a bit simpler, see the patch.
This routine was originally named absClusterPath(), but even at its
origin point (fe59e56) this argument has never been used. So no
objections to clean up that.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Dead code in adminpack
@ 2020-02-14 03:44 Michael Paquier <michael@paquier.xyz>
parent: Julien Rouhaud <rjuju123@gmail.com>
0 siblings, 0 replies; 4+ messages in thread
From: Michael Paquier @ 2020-02-14 03:44 UTC (permalink / raw)
To: Julien Rouhaud <rjuju123@gmail.com>; +Cc: Antonin Houska <ah@cybertec.at>; pgsql-hackers
On Thu, Feb 13, 2020 at 12:41:46PM +0100, Julien Rouhaud wrote:
> On Thu, Feb 13, 2020 at 12:14 PM Antonin Houska <ah@cybertec.at> wrote:
>> I've noticed that convert_and_check_filename() is always passed false for the
>> "logAllowed" argument - someone probably forgot to remove the argument when it
>> was decided that log files are no longer accepted. If the argument was removed,
>> the function would become a bit simpler, see the patch.
>
> Indeed, and actually I don't see when this codepath was reachable.
>
> Patch LGTM.
Thanks, applied.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2020-02-14 03:44 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-02-13 11:15 Dead code in adminpack Antonin Houska <ah@cybertec.at>
2020-02-13 11:41 ` Julien Rouhaud <rjuju123@gmail.com>
2020-02-14 03:44 ` Michael Paquier <michael@paquier.xyz>
2020-02-13 11:45 ` Michael Paquier <michael@paquier.xyz>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox