Add some missing exit() calls in error paths for various binaries

michaelpq · michaelpq · commit 856de3b39cf6 · 2021-07-29T11:42:58.000+09:00
The following changes are done: - In pg_archivecleanup, the cleanup of older WAL segments would never fail immediately. - In pgbench, the initialization of a thread barrier would not fail hard. - In pg_recvlogical, a stat() failure never got the call. - In pg_basebackup, two chmod() reported a failure without exit()'ing when unpacking some tar data freshly received. It may be possible to continue writing some data even after this failure, but that could be confusing to the user at the end. These are arguably bugs, but they would happen for code paths where a failure is unlikely going to happen, so no backpatch is done. Reviewed-by: Robert Haas, Fabien Coelho Discussion: https://postgr.es/m/YQDMdB+B68yePFeT@paquier.xyz
diff --git a/src/bin/pg_archivecleanup/pg_archivecleanup.c b/src/bin/pg_archivecleanup/pg_archivecleanup.c
@@ -151,21 +151,30 @@ CleanupPriorWALFiles(void)
 				{
 					pg_log_error("could not remove file \"%s\": %m",
 								 WALFilePath);
-					break;
+					exit(1);
 				}
 			}
 		}
 
 		if (errno)
+		{
 			pg_log_error("could not read archive location \"%s\": %m",
 						 archiveLocation);
+			exit(1);
+		}
 		if (closedir(xldir))
+		{
 			pg_log_error("could not close archive location \"%s\": %m",
 						 archiveLocation);
+			exit(1);
+		}
 	}
 	else
+	{
 		pg_log_error("could not open archive location \"%s\": %m",
 					 archiveLocation);
+		exit(1);
+	}
 }
 
 /*
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
@@ -1626,8 +1626,11 @@ ReceiveTarAndUnpackCopyChunk(size_t r, char *copybuf, void *callback_data)
 				}
 #ifndef WIN32
 				if (chmod(state->filename, (mode_t) filemode))
+				{
 					pg_log_error("could not set permissions on directory \"%s\": %m",
 								 state->filename);
+					exit(1);
+				}
 #endif
 			}
 			else if (copybuf[156] == '2')
@@ -1676,8 +1679,11 @@ ReceiveTarAndUnpackCopyChunk(size_t r, char *copybuf, void *callback_data)
 
 #ifndef WIN32
 		if (chmod(state->filename, (mode_t) filemode))
+		{
 			pg_log_error("could not set permissions on file \"%s\": %m",
 						 state->filename);
+			exit(1);
+		}
 #endif
 
 		if (state->current_len_left == 0)
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -341,7 +341,10 @@ StreamLogicalLog(void)
 			}
 
 			if (fstat(outfd, &statbuf) != 0)
+			{
 				pg_log_error("could not stat file \"%s\": %m", outfile);
+				goto error;
+			}
 
 			output_isfile = S_ISREG(statbuf.st_mode) && !isatty(outfd);
 		}
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
@@ -6469,7 +6469,10 @@ main(int argc, char **argv)
 
 	errno = THREAD_BARRIER_INIT(&barrier, nthreads);
 	if (errno != 0)
+	{
 		pg_log_fatal("could not initialize barrier: %m");
+		exit(1);
+	}
 
 #ifdef ENABLE_THREAD_SAFETY
 	/* start all threads but thread 0 which is executed directly later */

Original file line number	Diff line number	Diff line change
`@@ -151,21 +151,30 @@ CleanupPriorWALFiles(void)`
`151`	`151`	`{`
`152`	`152`	`pg_log_error("could not remove file \"%s\": %m",`
`153`	`153`	`WALFilePath);`
`154`		`- break;`
	`154`	`+ exit(1);`
`155`	`155`	`}`
`156`	`156`	`}`
`157`	`157`	`}`
`158`	`158`
`159`	`159`	`if (errno)`
	`160`	`+ {`
`160`	`161`	`pg_log_error("could not read archive location \"%s\": %m",`
`161`	`162`	`archiveLocation);`
	`163`	`+ exit(1);`
	`164`	`+ }`
`162`	`165`	`if (closedir(xldir))`
	`166`	`+ {`
`163`	`167`	`pg_log_error("could not close archive location \"%s\": %m",`
`164`	`168`	`archiveLocation);`
	`169`	`+ exit(1);`
	`170`	`+ }`
`165`	`171`	`}`
`166`	`172`	`else`
	`173`	`+ {`
`167`	`174`	`pg_log_error("could not open archive location \"%s\": %m",`
`168`	`175`	`archiveLocation);`
	`176`	`+ exit(1);`
	`177`	`+ }`
`169`	`178`	`}`
`170`	`179`
`171`	`180`	`/*`
Original file line number	Diff line number	Diff line change
`@@ -341,7 +341,10 @@ StreamLogicalLog(void)`
`341`	`341`	`}`
`342`	`342`
`343`	`343`	`if (fstat(outfd, &statbuf) != 0)`
	`344`	`+ {`
`344`	`345`	`pg_log_error("could not stat file \"%s\": %m", outfile);`
	`346`	`+ goto error;`
	`347`	`+ }`
`345`	`348`
`346`	`349`	`output_isfile = S_ISREG(statbuf.st_mode) && !isatty(outfd);`
`347`	`350`	`}`