Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit088c065

Browse files
committed
pg_upgrade: Fix exec_prog API to be less flaky
The previous signature made it very easy to pass something other thanthe printf-format specifier in the corresponding position, without anywarning from the compiler.While at it, move some of the escaping, redirecting and quotingresponsibilities from the callers into exec_prog() itself. This makesthe callsites cleaner.
1 parent34c0204 commit088c065

File tree

6 files changed

+96
-113
lines changed

6 files changed

+96
-113
lines changed

‎contrib/pg_upgrade/check.c

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -183,13 +183,10 @@ issue_warnings(char *sequence_script_file_name)
183183
if (sequence_script_file_name)
184184
{
185185
prep_status("Adjusting sequences");
186-
exec_prog(true, true,UTILITY_LOG_FILE,NULL,
187-
SYSTEMQUOTE"\"%s/psql\" --echo-queries "
188-
"--set ON_ERROR_STOP=on "
189-
"--no-psqlrc --port %d --username \"%s\" "
190-
"-f \"%s\" --dbname template1 >> \"%s\" 2>&1"SYSTEMQUOTE,
186+
exec_prog(UTILITY_LOG_FILE,NULL, true,
187+
"\"%s/psql\" "EXEC_PSQL_ARGS" --port %d --username \"%s\" -f \"%s\"",
191188
new_cluster.bindir,new_cluster.port,os_info.user,
192-
sequence_script_file_name,UTILITY_LOG_FILE);
189+
sequence_script_file_name);
193190
unlink(sequence_script_file_name);
194191
check_ok();
195192
}

‎contrib/pg_upgrade/dump.c

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,11 @@ generate_old_dump(void)
2323
* --binary-upgrade records the width of dropped columns in pg_class, and
2424
* restores the frozenid's for databases and relations.
2525
*/
26-
exec_prog(true, true,UTILITY_LOG_FILE,NULL,
27-
SYSTEMQUOTE"\"%s/pg_dumpall\" --port %d --username \"%s\" "
28-
"--schema-only --binary-upgrade %s > \"%s\" 2>> \"%s\""
29-
SYSTEMQUOTE,new_cluster.bindir,old_cluster.port,os_info.user,
26+
exec_prog(UTILITY_LOG_FILE,NULL, true,
27+
"\"%s/pg_dumpall\" --port %d --username \"%s\" --schema-only --binary-upgrade %s -f %s",
28+
new_cluster.bindir,old_cluster.port,os_info.user,
3029
log_opts.verbose ?"--verbose" :"",
31-
ALL_DUMP_FILE,UTILITY_LOG_FILE);
30+
ALL_DUMP_FILE);
3231
check_ok();
3332
}
3433

‎contrib/pg_upgrade/exec.c

Lines changed: 42 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -26,77 +26,81 @@ static intwin32_check_directory_write_permissions(void);
2626

2727
/*
2828
* exec_prog()
29+
*Execute an external program with stdout/stderr redirected, and report
30+
*errors
2931
*
30-
*Formats a command from the given argument list and executes that
31-
*command. If the command executes, exec_prog() returns 1 otherwise
32-
*exec_prog() logs an error message and returns 0. Either way, the command
33-
*line to be executed is saved to the specified log file.
32+
* Formats a command from the given argument list, logs it to the log file,
33+
* and attempts to execute that command. If the command executes
34+
* successfully, exec_prog() returns true.
3435
*
35-
*If throw_error is TRUE, this function will throw a PG_FATAL error
36-
*instead of returning should an error occur. The command it appended
37-
*to log_file; opt_log_file is used in error messages.
36+
* If the command fails, an error message is saved to the specified log_file.
37+
* If throw_error is true, this raises a PG_FATAL error and pg_upgrade
38+
* terminates; otherwise it is just reported as PG_REPORT and exec_prog()
39+
* returns false.
3840
*/
39-
int
40-
exec_prog(boolthrow_error,boolis_priv,constchar*log_file,
41-
constchar*opt_log_file,constchar*fmt,...)
41+
bool
42+
exec_prog(constchar*log_file,constchar*opt_log_file,
43+
boolthrow_error,constchar*fmt,...)
4244
{
43-
va_listargs;
4445
intresult;
45-
intretval;
46-
charcmd[MAXPGPATH];
46+
intwritten;
47+
#defineMAXCMDLEN (2 * MAXPGPATH)
48+
charcmd[MAXCMDLEN];
4749
mode_told_umask=0;
4850
FILE*log;
51+
va_listap;
4952

50-
if (is_priv)
51-
old_umask=umask(S_IRWXG |S_IRWXO);
53+
old_umask=umask(S_IRWXG |S_IRWXO);
5254

53-
va_start(args,fmt);
54-
vsnprintf(cmd,MAXPGPATH,fmt,args);
55-
va_end(args);
55+
written=strlcpy(cmd,SYSTEMQUOTE,strlen(SYSTEMQUOTE));
56+
va_start(ap,fmt);
57+
written+=vsnprintf(cmd+written,MAXCMDLEN-written,fmt,ap);
58+
va_end(ap);
59+
if (written >=MAXCMDLEN)
60+
pg_log(PG_FATAL,"command too long\n");
61+
written+=snprintf(cmd+written,MAXCMDLEN-written,
62+
" >> \"%s\" 2>&1"SYSTEMQUOTE,log_file);
63+
if (written >=MAXCMDLEN)
64+
pg_log(PG_FATAL,"command too long\n");
5665

5766
if ((log=fopen_priv(log_file,"a+"))==NULL)
5867
pg_log(PG_FATAL,"cannot write to log file %s\n",log_file);
5968
pg_log(PG_VERBOSE,"%s\n",cmd);
6069
fprintf(log,"command: %s\n",cmd);
70+
6171
/*
62-
*In Windows, we must closethen reopenthe log file so the file is
63-
*not open while the command is running, or we get a share violation.
72+
*In Windows, we must close the log fileat this pointso the file is not
73+
* open while the command is running, or we get a share violation.
6474
*/
6575
fclose(log);
6676

6777
result=system(cmd);
6878

69-
if (is_priv)
70-
umask(old_umask);
79+
umask(old_umask);
7180

7281
if (result!=0)
7382
{
74-
charopt_string[MAXPGPATH];
75-
76-
/* Create string for optional second log file */
77-
if (opt_log_file)
78-
snprintf(opt_string,sizeof(opt_string)," or \"%s\"",opt_log_file);
79-
else
80-
opt_string[0]='\0';
81-
8283
report_status(PG_REPORT,"*failure*");
8384
fflush(stdout);
8485
pg_log(PG_VERBOSE,"There were problems executing \"%s\"\n",cmd);
85-
pg_log(throw_error ?PG_FATAL :PG_REPORT,
86-
"Consult the last few lines of \"%s\"%s for\n"
87-
"the probable cause of the failure.\n",
88-
log_file,opt_string);
89-
retval=1;
86+
if (opt_log_file)
87+
pg_log(throw_error ?PG_FATAL :PG_REPORT,
88+
"Consult the last few lines of \"%s\" or \"%s\" for\n"
89+
"the probable cause of the failure.\n",
90+
log_file,opt_log_file);
91+
else
92+
pg_log(throw_error ?PG_FATAL :PG_REPORT,
93+
"Consult the last few lines of \"%s\" for\n"
94+
"the probable cause of the failure.\n",
95+
log_file);
9096
}
91-
else
92-
retval=0;
9397

9498
if ((log=fopen_priv(log_file,"a+"))==NULL)
9599
pg_log(PG_FATAL,"cannot write to log file %s\n",log_file);
96100
fprintf(log,"\n\n");
97101
fclose(log);
98102

99-
returnretval;
103+
returnresult==0;
100104
}
101105

102106

‎contrib/pg_upgrade/pg_upgrade.c

Lines changed: 26 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -140,11 +140,10 @@ main(int argc, char **argv)
140140
* because there is no need to have the schema load use new oids.
141141
*/
142142
prep_status("Setting next OID for new cluster");
143-
exec_prog(true, true,UTILITY_LOG_FILE,NULL,
144-
SYSTEMQUOTE"\"%s/pg_resetxlog\" -o %u \"%s\" >> \"%s\" 2>&1"
145-
SYSTEMQUOTE,
143+
exec_prog(UTILITY_LOG_FILE,NULL, true,
144+
"\"%s/pg_resetxlog\" -o %u \"%s\"",
146145
new_cluster.bindir,old_cluster.controldata.chkpnt_nxtoid,
147-
new_cluster.pgdata,UTILITY_LOG_FILE);
146+
new_cluster.pgdata);
148147
check_ok();
149148

150149
create_script_for_cluster_analyze(&analyze_script_file_name);
@@ -211,11 +210,10 @@ prepare_new_cluster(void)
211210
* --analyze so autovacuum doesn't update statistics later
212211
*/
213212
prep_status("Analyzing all rows in the new cluster");
214-
exec_prog(true, true,UTILITY_LOG_FILE,NULL,
215-
SYSTEMQUOTE"\"%s/vacuumdb\" --port %d --username \"%s\" "
216-
"--all --analyze %s >> \"%s\" 2>&1"SYSTEMQUOTE,
213+
exec_prog(UTILITY_LOG_FILE,NULL, true,
214+
"\"%s/vacuumdb\" --port %d --username \"%s\" --all --analyze %s",
217215
new_cluster.bindir,new_cluster.port,os_info.user,
218-
log_opts.verbose ?"--verbose" :"",UTILITY_LOG_FILE);
216+
log_opts.verbose ?"--verbose" :"");
219217
check_ok();
220218

221219
/*
@@ -225,11 +223,10 @@ prepare_new_cluster(void)
225223
* later.
226224
*/
227225
prep_status("Freezing all rows on the new cluster");
228-
exec_prog(true, true,UTILITY_LOG_FILE,NULL,
229-
SYSTEMQUOTE"\"%s/vacuumdb\" --port %d --username \"%s\" "
230-
"--all --freeze %s >> \"%s\" 2>&1"SYSTEMQUOTE,
226+
exec_prog(UTILITY_LOG_FILE,NULL, true,
227+
"\"%s/vacuumdb\" --port %d --username \"%s\" --all --freeze %s",
231228
new_cluster.bindir,new_cluster.port,os_info.user,
232-
log_opts.verbose ?"--verbose" :"",UTILITY_LOG_FILE);
229+
log_opts.verbose ?"--verbose" :"");
233230
check_ok();
234231

235232
get_pg_database_relfilenode(&new_cluster);
@@ -263,14 +260,10 @@ prepare_new_databases(void)
263260
* support functions in template1 but pg_dumpall creates database using
264261
* the template0 template.
265262
*/
266-
exec_prog(true, true,RESTORE_LOG_FILE,NULL,
267-
SYSTEMQUOTE"\"%s/psql\" --echo-queries "
268-
"--set ON_ERROR_STOP=on "
269-
/* --no-psqlrc prevents AUTOCOMMIT=off */
270-
"--no-psqlrc --port %d --username \"%s\" "
271-
"-f \"%s\" --dbname template1 >> \"%s\" 2>&1"SYSTEMQUOTE,
263+
exec_prog(RESTORE_LOG_FILE,NULL, true,
264+
"\"%s/psql\" "EXEC_PSQL_ARGS" --port %d --username \"%s\" -f \"%s\"",
272265
new_cluster.bindir,new_cluster.port,os_info.user,
273-
GLOBALS_DUMP_FILE,RESTORE_LOG_FILE);
266+
GLOBALS_DUMP_FILE);
274267
check_ok();
275268

276269
/* we load this to get a current list of databases */
@@ -296,13 +289,10 @@ create_new_objects(void)
296289
check_ok();
297290

298291
prep_status("Restoring database schema to new cluster");
299-
exec_prog(true, true,RESTORE_LOG_FILE,NULL,
300-
SYSTEMQUOTE"\"%s/psql\" --echo-queries "
301-
"--set ON_ERROR_STOP=on "
302-
"--no-psqlrc --port %d --username \"%s\" "
303-
"-f \"%s\" --dbname template1 >> \"%s\" 2>&1"SYSTEMQUOTE,
292+
exec_prog(RESTORE_LOG_FILE,NULL, true,
293+
"\"%s/psql\" "EXEC_PSQL_ARGS" --port %d --username \"%s\" -f \"%s\"",
304294
new_cluster.bindir,new_cluster.port,os_info.user,
305-
DB_DUMP_FILE,RESTORE_LOG_FILE);
295+
DB_DUMP_FILE);
306296
check_ok();
307297

308298
/* regenerate now that we have objects in the databases */
@@ -331,16 +321,14 @@ copy_subdir_files(char *subdir)
331321

332322
prep_status("Copying old %s to new server",subdir);
333323

334-
exec_prog(true, false,UTILITY_LOG_FILE,NULL,
324+
exec_prog(UTILITY_LOG_FILE,NULL, true,
335325
#ifndefWIN32
336-
SYSTEMQUOTE"%s \"%s\" \"%s\" >> \"%s\" 2>&1"SYSTEMQUOTE,
337-
"cp -Rf",
326+
"cp -Rf \"%s\" \"%s\"",
338327
#else
339328
/* flags: everything, no confirm, quiet, overwrite read-only */
340-
SYSTEMQUOTE"%s \"%s\" \"%s\\\" >> \"%s\" 2>&1"SYSTEMQUOTE,
341-
"xcopy /e /y /q /r",
329+
"xcopy /e /y /q /r \"%s\" \"%s\\\"",
342330
#endif
343-
old_path,new_path,UTILITY_LOG_FILE);
331+
old_path,new_path);
344332

345333
check_ok();
346334
}
@@ -353,22 +341,18 @@ copy_clog_xlog_xid(void)
353341

354342
/* set the next transaction id of the new cluster */
355343
prep_status("Setting next transaction ID for new cluster");
356-
exec_prog(true, true,UTILITY_LOG_FILE,NULL,
357-
SYSTEMQUOTE
358-
"\"%s/pg_resetxlog\" -f -x %u \"%s\" >> \"%s\" 2>&1"
359-
SYSTEMQUOTE,new_cluster.bindir,
360-
old_cluster.controldata.chkpnt_nxtxid,
361-
new_cluster.pgdata,UTILITY_LOG_FILE);
344+
exec_prog(UTILITY_LOG_FILE,NULL, true,
345+
"\"%s/pg_resetxlog\" -f -x %u \"%s\"",
346+
new_cluster.bindir,old_cluster.controldata.chkpnt_nxtxid,
347+
new_cluster.pgdata);
362348
check_ok();
363349

364350
/* now reset the wal archives in the new cluster */
365351
prep_status("Resetting WAL archives");
366-
exec_prog(true, true,UTILITY_LOG_FILE,NULL,
367-
SYSTEMQUOTE
368-
"\"%s/pg_resetxlog\" -l %s \"%s\" >> \"%s\" 2>&1"
369-
SYSTEMQUOTE,new_cluster.bindir,
352+
exec_prog(UTILITY_LOG_FILE,NULL, true,
353+
"\"%s/pg_resetxlog\" -l %s \"%s\"",new_cluster.bindir,
370354
old_cluster.controldata.nextxlogfile,
371-
new_cluster.pgdata,UTILITY_LOG_FILE);
355+
new_cluster.pgdata);
372356
check_ok();
373357
}
374358

‎contrib/pg_upgrade/pg_upgrade.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -316,10 +316,11 @@ voidsplit_old_dump(void);
316316

317317
/* exec.c */
318318

319-
int
320-
exec_prog(boolthrow_error,boolis_priv,constchar*log_file,
321-
constchar*opt_log_file,constchar*cmd,...)
322-
__attribute__((format(PG_PRINTF_ATTRIBUTE,5,6)));
319+
#defineEXEC_PSQL_ARGS "--echo-queries --set ON_ERROR_STOP=on --no-psqlrc --dbname=template1"
320+
bool
321+
exec_prog(constchar*log_file,constchar*opt_log_file,
322+
boolthrow_error,constchar*fmt,...)
323+
__attribute__((format(PG_PRINTF_ATTRIBUTE,4,5)));
323324
voidverify_directories(void);
324325
boolis_server_running(constchar*datadir);
325326

‎contrib/pg_upgrade/server.c

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ start_postmaster(ClusterInfo *cluster)
143143
charcmd[MAXPGPATH];
144144
PGconn*conn;
145145
boolexit_hook_registered= false;
146-
intpg_ctl_return=0;
146+
boolpg_ctl_return=false;
147147

148148
if (!exit_hook_registered)
149149
{
@@ -159,22 +159,23 @@ start_postmaster(ClusterInfo *cluster)
159159
* not touch them.
160160
*/
161161
snprintf(cmd,sizeof(cmd),
162-
SYSTEMQUOTE"\"%s/pg_ctl\" -w -l \"%s\" -D \"%s\" "
163-
"-o \"-p %d %s %s\" start >> \"%s\" 2>&1"SYSTEMQUOTE,
162+
"\"%s/pg_ctl\" -w -l \"%s\" -D \"%s\" -o \"-p %d %s %s\" start",
164163
cluster->bindir,SERVER_LOG_FILE,cluster->pgconfig,cluster->port,
165164
(cluster->controldata.cat_ver >=
166165
BINARY_UPGRADE_SERVER_FLAG_CAT_VER) ?"-b" :
167166
"-c autovacuum=off -c autovacuum_freeze_max_age=2000000000",
168-
cluster->pgopts ?cluster->pgopts :"",SERVER_START_LOG_FILE);
167+
cluster->pgopts ?cluster->pgopts :"");
169168

170169
/*
171170
* Don't throw an error right away, let connecting throw the error because
172171
* it might supply a reason for the failure.
173172
*/
174-
pg_ctl_return=exec_prog(false, true,SERVER_START_LOG_FILE,
175-
/* pass both file names if the differ */
176-
(strcmp(SERVER_LOG_FILE,SERVER_START_LOG_FILE)!=0) ?
173+
pg_ctl_return=exec_prog(SERVER_START_LOG_FILE,
174+
/* pass both file names if they differ */
175+
(strcmp(SERVER_LOG_FILE,
176+
SERVER_START_LOG_FILE)!=0) ?
177177
SERVER_LOG_FILE :NULL,
178+
false,
178179
"%s",cmd);
179180

180181
/* Check to see if we can connect to the server; if not, report it. */
@@ -185,13 +186,14 @@ start_postmaster(ClusterInfo *cluster)
185186
PQerrorMessage(conn));
186187
if (conn)
187188
PQfinish(conn);
188-
pg_log(PG_FATAL,"could not connect to %s postmaster started with the command: %s\n",
189+
pg_log(PG_FATAL,"could not connect to %s postmaster started with the command:\n"
190+
"%s\n",
189191
CLUSTER_NAME(cluster),cmd);
190192
}
191193
PQfinish(conn);
192194

193195
/* If the connection didn't fail, fail now */
194-
if (pg_ctl_return!=0)
196+
if (!pg_ctl_return)
195197
pg_log(PG_FATAL,"pg_ctl failed to start the %s server, or connection failed\n",
196198
CLUSTER_NAME(cluster));
197199

@@ -202,7 +204,6 @@ start_postmaster(ClusterInfo *cluster)
202204
void
203205
stop_postmaster(boolfast)
204206
{
205-
charcmd[MAXPGPATH];
206207
ClusterInfo*cluster;
207208

208209
if (os_info.running_cluster==&old_cluster)
@@ -212,14 +213,11 @@ stop_postmaster(bool fast)
212213
else
213214
return;/* no cluster running */
214215

215-
snprintf(cmd,sizeof(cmd),
216-
SYSTEMQUOTE"\"%s/pg_ctl\" -w -D \"%s\" -o \"%s\" "
217-
"%s stop >> \"%s\" 2>&1"SYSTEMQUOTE,
218-
cluster->bindir,cluster->pgconfig,
219-
cluster->pgopts ?cluster->pgopts :"",
220-
fast ?"-m fast" :"",SERVER_STOP_LOG_FILE);
221-
222-
exec_prog(fast ? false : true, true,SERVER_STOP_LOG_FILE,NULL,"%s",cmd);
216+
exec_prog(SERVER_STOP_LOG_FILE,NULL, !fast,
217+
"\"%s/pg_ctl\" -w -D \"%s\" -o \"%s\" %s stop",
218+
cluster->bindir,cluster->pgconfig,
219+
cluster->pgopts ?cluster->pgopts :"",
220+
fast ?"-m fast" :"");
223221

224222
os_info.running_cluster=NULL;
225223
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp