"buffer too small" or "path too long"?

Started by Kyotaro Horiguchiover 3 years ago13 messages

horikyota.ntt@gmail.com

over 3 years ago

1 attachment(s)

Recently we added the error messages "buffer for root directory too
small" and siblings to pg_upgrade. This means "<new_cluster's
pgdata>/pg_upgrade_output.d" was longer than MAXPGPATH.

I feel that the "root directory" is obscure here, and moreover "buffer
is too small" looks pointless since no user can do something on the
buffer length. At least I can't read out from the message concretely
what I should do next..

The root cause of the errors is that the user-provided directory path
of new cluster's root was too long. Anywhich one of the four buffers
is overflowed, it doesn't makes any difference for users and doesn't
offer any further detail to suppoerters/developers. I see "output
directory path of new cluster too long" clear enough.

Above all, this change reduces the number of messages that need
translation:)

# And the messages are missing trailing line breaks.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

Attachments:

0001-Revise-some-error-messages-of-pg_upgrade.patchtext/x-patch; charset=us-asciiDownload

From 5fffa05aba3ae2aed36ba76e6edee5b465713be5 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Mon, 13 Jun 2022 11:49:12 +0900
Subject: [PATCH] Revise some error messages of pg_upgrade

Some newly added error messages are too detailed and hardly give users
information on what was wrong on their side.  And they are missing
trailing newlines.  There's no point in differentiating the messages
each other so replace them with one message clear to users.
---
 src/bin/pg_upgrade/pg_upgrade.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index ccb048ab2e..737d962658 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -228,7 +228,7 @@ make_outputdirs(char *pgdata)
 	log_opts.rootdir = (char *) pg_malloc0(MAXPGPATH);
 	len = snprintf(log_opts.rootdir, MAXPGPATH, "%s/%s", pgdata, BASE_OUTPUTDIR);
 	if (len >= MAXPGPATH)
-		pg_fatal("buffer for root directory too small");
+		pg_fatal("directory path for new cluster too long\n");
 
 	/* BASE_OUTPUTDIR/$timestamp/ */
 	gettimeofday(&time, NULL);
@@ -241,21 +241,21 @@ make_outputdirs(char *pgdata)
 	len = snprintf(log_opts.basedir, MAXPGPATH, "%s/%s", log_opts.rootdir,
 				   timebuf);
 	if (len >= MAXPGPATH)
-		pg_fatal("buffer for base directory too small");
+		pg_fatal("directory path for new cluster too long\n");
 
 	/* BASE_OUTPUTDIR/$timestamp/dump/ */
 	log_opts.dumpdir = (char *) pg_malloc0(MAXPGPATH);
 	len = snprintf(log_opts.dumpdir, MAXPGPATH, "%s/%s/%s", log_opts.rootdir,
 				   timebuf, DUMP_OUTPUTDIR);
 	if (len >= MAXPGPATH)
-		pg_fatal("buffer for dump directory too small");
+		pg_fatal("directory path for new cluster too long\n");
 
 	/* BASE_OUTPUTDIR/$timestamp/log/ */
 	log_opts.logdir = (char *) pg_malloc0(MAXPGPATH);
 	len = snprintf(log_opts.logdir, MAXPGPATH, "%s/%s/%s", log_opts.rootdir,
 				   timebuf, LOG_OUTPUTDIR);
 	if (len >= MAXPGPATH)
-		pg_fatal("buffer for log directory too small");
+		pg_fatal("directory path for new cluster too long\n");
 
 	/*
 	 * Ignore the error case where the root path exists, as it is kept the
-- 
2.31.1