From 4e54d231aecb68cf64daf56ab7851c699719762e Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 13 Jun 2022 14:28:05 -0400 Subject: [PATCH] pg_upgrade: further tweaking of make_outputdirs(). Use the same error message for all cases of pathname overrun, since users aren't going to much care which one was too long. Add missing newline to said error (as pg_upgrade's version of pg_fatal requires that). Add pathname overrun checks for the individual log files, not just the directories. Remove initial newline in log files; the new scheme here guarantees that we'll never be appending to an old file. Kyotaro Horiguchi and Tom Lane Discussion: https://postgr.es/m/20220613.120551.729848632120189555.horikyota.ntt@gmail.com --- src/bin/pg_upgrade/pg_upgrade.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c index ccb048ab2e5..265d8294906 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 is 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 is 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 is 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 is too long\n"); /* * Ignore the error case where the root path exists, as it is kept the @@ -270,21 +270,25 @@ make_outputdirs(char *pgdata) if (mkdir(log_opts.logdir, pg_dir_create_mode) < 0) pg_fatal("could not create directory \"%s\": %m\n", log_opts.logdir); - snprintf(filename_path, sizeof(filename_path), "%s/%s", log_opts.logdir, - INTERNAL_LOG_FILE); + len = snprintf(filename_path, sizeof(filename_path), "%s/%s", + log_opts.logdir, INTERNAL_LOG_FILE); + if (len >= sizeof(filename_path)) + pg_fatal("directory path for new cluster is too long\n"); + if ((log_opts.internal = fopen_priv(filename_path, "a")) == NULL) pg_fatal("could not open log file \"%s\": %m\n", filename_path); /* label start of upgrade in logfiles */ for (filename = output_files; *filename != NULL; filename++) { - snprintf(filename_path, sizeof(filename_path), "%s/%s", - log_opts.logdir, *filename); + len = snprintf(filename_path, sizeof(filename_path), "%s/%s", + log_opts.logdir, *filename); + if (len >= sizeof(filename_path)) + pg_fatal("directory path for new cluster is too long\n"); if ((fp = fopen_priv(filename_path, "a")) == NULL) pg_fatal("could not write to log file \"%s\": %m\n", filename_path); - /* Start with newline because we might be appending to a file. */ - fprintf(fp, "\n" + fprintf(fp, "-----------------------------------------------------------------\n" " pg_upgrade run on %s" "-----------------------------------------------------------------\n\n",