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

Commitf002ed2

Browse files
committed
Improve error reporting in pg_upgrade's file copying/linking/rewriting.
The previous design for this had copyFile(), linkFile(), andrewriteVisibilityMap() returning strerror strings, with the callerproducing one-size-fits-all error messages based on that. This made itimpossible to produce messages that described the failures with any degreeof precision, especially not short-read problems since those don't seterrno at all.Since pg_upgrade has no intention of continuing after any error in thisarea, let's fix this by just letting these functions call pg_fatal() forthemselves, making it easy for each point of failure to have a suitableerror message. Taking this approach also allows dropping cleanup codethat was unnecessary and was often rather sloppy about preserving errno.To not lose relevant info that was reported before, pass in the schema nameand table name of the current table so that they can be included in theerror reports.An additional problem was the use of getErrorText(), which was flat outwrong for all but a couple of call sites, because it unconditionally did"_dosmaperr(GetLastError())" on Windows. That's only appropriate whenreporting an error from a Windows-native API, which only a couple ofthe callers were actually doing. Thus, even the reported strerror stringwould be unrelated to the actual failure in many cases on Windows.To fix, get rid of getErrorText() altogether, and just have call sitesdo strerror(errno) instead, since that's the way all the rest of ourfrontend programs do it. Add back the _dosmaperr() calls in the twoplaces where that's actually appropriate.In passing, make assorted messages hew more closely to project styleguidelines, notably by removing initial capitals in not-complete-sentenceprimary error messages. (I didn't make any effort to clean up placesI didn't have another reason to touch, though.)Per discussion of a report from Thomas Kellerer. Back-patch to 9.6,but no further; given the relative infrequency of reports of problemshere, it's not clear it's worth adapting the patch to older branches.Patch by me, but with credit to Alvaro Herrera for spotting the issuewith getErrorText's misuse of _dosmaperr().Discussion: <nsjrbh$8li$1@blaine.gmane.org>
1 parent5afcd2a commitf002ed2

File tree

12 files changed

+132
-184
lines changed

12 files changed

+132
-184
lines changed

‎src/bin/pg_upgrade/check.c

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -431,8 +431,8 @@ create_script_for_cluster_analyze(char **analyze_script_file_name)
431431
SCRIPT_PREFIX,SCRIPT_EXT);
432432

433433
if ((script=fopen_priv(*analyze_script_file_name,"w"))==NULL)
434-
pg_fatal("Could not open file \"%s\": %s\n",
435-
*analyze_script_file_name,getErrorText());
434+
pg_fatal("could not open file \"%s\": %s\n",
435+
*analyze_script_file_name,strerror(errno));
436436

437437
#ifndefWIN32
438438
/* add shebang header */
@@ -486,8 +486,8 @@ create_script_for_cluster_analyze(char **analyze_script_file_name)
486486

487487
#ifndefWIN32
488488
if (chmod(*analyze_script_file_name,S_IRWXU)!=0)
489-
pg_fatal("Could not add execute permission to file \"%s\": %s\n",
490-
*analyze_script_file_name,getErrorText());
489+
pg_fatal("could not add execute permission to file \"%s\": %s\n",
490+
*analyze_script_file_name,strerror(errno));
491491
#endif
492492

493493
termPQExpBuffer(&user_specification);
@@ -559,8 +559,8 @@ create_script_for_old_cluster_deletion(char **deletion_script_file_name)
559559
prep_status("Creating script to delete old cluster");
560560

561561
if ((script=fopen_priv(*deletion_script_file_name,"w"))==NULL)
562-
pg_fatal("Could not open file \"%s\": %s\n",
563-
*deletion_script_file_name,getErrorText());
562+
pg_fatal("could not open file \"%s\": %s\n",
563+
*deletion_script_file_name,strerror(errno));
564564

565565
#ifndefWIN32
566566
/* add shebang header */
@@ -615,8 +615,8 @@ create_script_for_old_cluster_deletion(char **deletion_script_file_name)
615615

616616
#ifndefWIN32
617617
if (chmod(*deletion_script_file_name,S_IRWXU)!=0)
618-
pg_fatal("Could not add execute permission to file \"%s\": %s\n",
619-
*deletion_script_file_name,getErrorText());
618+
pg_fatal("could not add execute permission to file \"%s\": %s\n",
619+
*deletion_script_file_name,strerror(errno));
620620
#endif
621621

622622
check_ok();
@@ -819,8 +819,8 @@ check_for_isn_and_int8_passing_mismatch(ClusterInfo *cluster)
819819
{
820820
found= true;
821821
if (script==NULL&& (script=fopen_priv(output_path,"w"))==NULL)
822-
pg_fatal("Could not open file \"%s\": %s\n",
823-
output_path,getErrorText());
822+
pg_fatal("could not open file \"%s\": %s\n",
823+
output_path,strerror(errno));
824824
if (!db_used)
825825
{
826826
fprintf(script,"Database: %s\n",active_db->db_name);
@@ -922,8 +922,8 @@ check_for_reg_data_type_usage(ClusterInfo *cluster)
922922
{
923923
found= true;
924924
if (script==NULL&& (script=fopen_priv(output_path,"w"))==NULL)
925-
pg_fatal("Could not open file \"%s\": %s\n",
926-
output_path,getErrorText());
925+
pg_fatal("could not open file \"%s\": %s\n",
926+
output_path,strerror(errno));
927927
if (!db_used)
928928
{
929929
fprintf(script,"Database: %s\n",active_db->db_name);
@@ -1013,8 +1013,8 @@ check_for_jsonb_9_4_usage(ClusterInfo *cluster)
10131013
{
10141014
found= true;
10151015
if (script==NULL&& (script=fopen_priv(output_path,"w"))==NULL)
1016-
pg_fatal("Could not open file \"%s\": %s\n",
1017-
output_path,getErrorText());
1016+
pg_fatal("could not open file \"%s\": %s\n",
1017+
output_path,strerror(errno));
10181018
if (!db_used)
10191019
{
10201020
fprintf(script,"Database: %s\n",active_db->db_name);
@@ -1089,8 +1089,8 @@ get_bin_version(ClusterInfo *cluster)
10891089

10901090
if ((output=popen(cmd,"r"))==NULL||
10911091
fgets(cmd_output,sizeof(cmd_output),output)==NULL)
1092-
pg_fatal("Could not get pg_ctl version data using %s: %s\n",
1093-
cmd,getErrorText());
1092+
pg_fatal("could not get pg_ctl version data using %s: %s\n",
1093+
cmd,strerror(errno));
10941094

10951095
pclose(output);
10961096

‎src/bin/pg_upgrade/controldata.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,8 @@ get_control_data(ClusterInfo *cluster, bool live_check)
119119
fflush(stderr);
120120

121121
if ((output=popen(cmd,"r"))==NULL)
122-
pg_fatal("Could not get control data using %s: %s\n",
123-
cmd,getErrorText());
122+
pg_fatal("could not get control data using %s: %s\n",
123+
cmd,strerror(errno));
124124

125125
/* Only in <= 9.2 */
126126
if (GET_MAJOR_VERSION(cluster->major_version) <=902)

‎src/bin/pg_upgrade/exec.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ pid_lock_file_exists(const char *datadir)
191191
/* ENOTDIR means we will throw a more useful error later */
192192
if (errno!=ENOENT&&errno!=ENOTDIR)
193193
pg_fatal("could not open file \"%s\" for reading: %s\n",
194-
path,getErrorText());
194+
path,strerror(errno));
195195

196196
return false;
197197
}
@@ -285,7 +285,7 @@ check_data_dir(const char *pg_data)
285285

286286
if (stat(subDirName,&statBuf)!=0)
287287
report_status(PG_FATAL,"check for \"%s\" failed: %s\n",
288-
subDirName,getErrorText());
288+
subDirName,strerror(errno));
289289
elseif (!S_ISDIR(statBuf.st_mode))
290290
report_status(PG_FATAL,"%s is not a directory\n",
291291
subDirName);
@@ -309,7 +309,7 @@ check_bin_dir(ClusterInfo *cluster)
309309
/* check bindir */
310310
if (stat(cluster->bindir,&statBuf)!=0)
311311
report_status(PG_FATAL,"check for \"%s\" failed: %s\n",
312-
cluster->bindir,getErrorText());
312+
cluster->bindir,strerror(errno));
313313
elseif (!S_ISDIR(statBuf.st_mode))
314314
report_status(PG_FATAL,"%s is not a directory\n",
315315
cluster->bindir);
@@ -352,7 +352,7 @@ validate_exec(const char *dir, const char *cmdName)
352352
*/
353353
if (stat(path,&buf)<0)
354354
pg_fatal("check for \"%s\" failed: %s\n",
355-
path,getErrorText());
355+
path,strerror(errno));
356356
elseif (!S_ISREG(buf.st_mode))
357357
pg_fatal("check for \"%s\" failed: not an executable file\n",
358358
path);

0 commit comments

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