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

Commit2a11b18

Browse files
committed
Clean up assorted messiness around AllocateDir() usage.
This patch fixes a couple of low-probability bugs that could lead toreporting an irrelevant errno value (and hence possibly a wrong SQLSTATE)concerning directory-open or file-open failures. It also fixes placeswhere we took shortcuts in reporting such errors, either by using eloginstead of ereport or by using ereport but forgetting to specify anerrcode. And it eliminates a lot of just plain redundant error-handlingcode.In service of all this, export fd.c's formerly-static functionReadDirExtended, so that external callers can make use of the codingpatterndir = AllocateDir(path);while ((de = ReadDirExtended(dir, path, LOG)) != NULL)if they'd like to treat directory-open failures as mere LOG conditionsrather than errors. Also fix FreeDir to be a no-op if we reach itwith dir == NULL, as such a coding pattern would cause.Then, remove code at many call sites that was throwing an error or logmessage for AllocateDir failure, as ReadDir or ReadDirExtended can handlethat job just fine. Aside from being a net code savings, this gets rid ofa lot of not-quite-up-to-snuff reports, as mentioned above. (In someplaces these changes result in replacing a custom error message such as"could not open tablespace directory" with more generic wording "could notopen directory", but it was agreed that the custom wording buys little aslong as we report the directory name.) In some other call sites where wecan't just remove code, change the error reports to be fullyproject-style-compliant.Also reorder code in restoreTwoPhaseData that was acquiring a lockbetween AllocateDir and ReadDir; in the unlikely but surely notimpossible case that LWLockAcquire changes errno, AllocateDir failureswould be misreported. There is no great value in opening the directorybefore acquiring TwoPhaseStateLock, so just do it in the other order.Also fix CheckXLogRemoved to guarantee that it preserves errno,as quite a number of call sites are implicitly assuming. (Again,it's unlikely but I think not impossible that errno could changeduring a SpinLockAcquire. If so, this function was broken for itsown purposes as well as breaking callers.)And change a few places that were using not-per-project-style messages,such as "could not read directory" when "could not open directory" ismore correct.Back-patch the exporting of ReadDirExtended, in case we have occasionto back-patch some fix that makes use of it; it's not needed right nowbut surely making it global is pretty harmless. Also back-patch therestoreTwoPhaseData and CheckXLogRemoved fixes. The rest of this isessentially cosmetic and need not get back-patched.Michael Paquier, with a bit of additional work by meDiscussion:https://postgr.es/m/CAB7nPqRpOCxjiirHmebEFhXVTK7V5Jvw4bz82p7Oimtsm3TyZA@mail.gmail.com
1 parentbf2b317 commit2a11b18

File tree

4 files changed

+35
-9
lines changed

4 files changed

+35
-9
lines changed

‎src/backend/access/transam/twophase.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1735,8 +1735,8 @@ restoreTwoPhaseData(void)
17351735
DIR*cldir;
17361736
structdirent*clde;
17371737

1738-
cldir=AllocateDir(TWOPHASE_DIR);
17391738
LWLockAcquire(TwoPhaseStateLock,LW_EXCLUSIVE);
1739+
cldir=AllocateDir(TWOPHASE_DIR);
17401740
while ((clde=ReadDir(cldir,TWOPHASE_DIR))!=NULL)
17411741
{
17421742
if (strlen(clde->d_name)==8&&

‎src/backend/access/transam/xlog.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3760,10 +3760,16 @@ PreallocXlogFiles(XLogRecPtr endptr)
37603760
* existed while the server has been running, as this function always
37613761
* succeeds if no WAL segments have been removed since startup.
37623762
* 'tli' is only used in the error message.
3763+
*
3764+
* Note: this function guarantees to keep errno unchanged on return.
3765+
* This supports callers that use this to possibly deliver a better
3766+
* error message about a missing file, while still being able to throw
3767+
* a normal file-access error afterwards, if this does return.
37633768
*/
37643769
void
37653770
CheckXLogRemoved(XLogSegNosegno,TimeLineIDtli)
37663771
{
3772+
intsave_errno=errno;
37673773
XLogSegNolastRemovedSegNo;
37683774

37693775
SpinLockAcquire(&XLogCtl->info_lck);
@@ -3775,11 +3781,13 @@ CheckXLogRemoved(XLogSegNo segno, TimeLineID tli)
37753781
charfilename[MAXFNAMELEN];
37763782

37773783
XLogFileName(filename,tli,segno);
3784+
errno=save_errno;
37783785
ereport(ERROR,
37793786
(errcode_for_file_access(),
37803787
errmsg("requested WAL segment %s has already been removed",
37813788
filename)));
37823789
}
3790+
errno=save_errno;
37833791
}
37843792

37853793
/*

‎src/backend/storage/file/fd.c

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,6 @@ static intFileAccess(File file);
304304
staticFileOpenTemporaryFileInTablespace(OidtblspcOid,boolrejectError);
305305
staticboolreserveAllocatedDesc(void);
306306
staticintFreeDesc(AllocateDesc*desc);
307-
staticstructdirent*ReadDirExtended(DIR*dir,constchar*dirname,intelevel);
308307

309308
staticvoidAtProcExit_Files(intcode,Datumarg);
310309
staticvoidCleanupTempFiles(boolisProcExit);
@@ -2335,6 +2334,10 @@ CloseTransientFile(int fd)
23352334
* necessary to open the directory, and with closing it after an elog.
23362335
* When done, call FreeDir rather than closedir.
23372336
*
2337+
* Returns NULL, with errno set, on failure. Note that failure detection
2338+
* is commonly left to the following call of ReadDir or ReadDirExtended;
2339+
* see the comments for ReadDir.
2340+
*
23382341
* Ideally this should be the *only* direct call of opendir() in the backend.
23392342
*/
23402343
DIR*
@@ -2397,8 +2400,8 @@ AllocateDir(const char *dirname)
23972400
*FreeDir(dir);
23982401
*
23992402
* since a NULL dir parameter is taken as indicating AllocateDir failed.
2400-
* (Make sure errnohasn'tbeenchangedsince AllocateDirif you use this
2401-
* shortcut.)
2403+
* (Make sure errnoisn't changedbetween AllocateDirand ReadDir if you
2404+
*use thisshortcut.)
24022405
*
24032406
* The pathname passed to AllocateDir must be passed to this routine too,
24042407
* but it is only used for error reporting.
@@ -2410,10 +2413,15 @@ ReadDir(DIR *dir, const char *dirname)
24102413
}
24112414

24122415
/*
2413-
* Alternate version that allows caller to specify the elevel for any
2414-
* error report. If elevel < ERROR, returns NULL on any error.
2416+
* Alternate version of ReadDir that allows caller to specify the elevel
2417+
* for any error report (whether it's reporting an initial failure of
2418+
* AllocateDir or a subsequent directory read failure).
2419+
*
2420+
* If elevel < ERROR, returns NULL after any error. With the normal coding
2421+
* pattern, this will result in falling out of the loop immediately as
2422+
* though the directory contained no (more) entries.
24152423
*/
2416-
staticstructdirent*
2424+
structdirent*
24172425
ReadDirExtended(DIR*dir,constchar*dirname,intelevel)
24182426
{
24192427
structdirent*dent;
@@ -2443,14 +2451,22 @@ ReadDirExtended(DIR *dir, const char *dirname, int elevel)
24432451
/*
24442452
* Close a directory opened with AllocateDir.
24452453
*
2446-
* Note we do not check closedir's return value --- it is up to the caller
2447-
* to handle close errors.
2454+
* Returns closedir's return value (with errno set if it's not 0).
2455+
* Note we do not check the return value --- it is up to the caller
2456+
* to handle close errors if wanted.
2457+
*
2458+
* Does nothing if dir == NULL; we assume that directory open failure was
2459+
* already reported if desired.
24482460
*/
24492461
int
24502462
FreeDir(DIR*dir)
24512463
{
24522464
inti;
24532465

2466+
/* Nothing to do if AllocateDir failed */
2467+
if (dir==NULL)
2468+
return0;
2469+
24542470
DO_DB(elog(LOG,"FreeDir: Allocated %d",numAllocatedDescs));
24552471

24562472
/* Remove dir from list of allocated dirs, if it's present */

‎src/include/storage/fd.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,8 @@ extern intClosePipeStream(FILE *file);
9191
/* Operations to allow use of the <dirent.h> library routines */
9292
externDIR*AllocateDir(constchar*dirname);
9393
externstructdirent*ReadDir(DIR*dir,constchar*dirname);
94+
externstructdirent*ReadDirExtended(DIR*dir,constchar*dirname,
95+
intelevel);
9496
externintFreeDir(DIR*dir);
9597

9698
/* Operations to allow use of a plain kernel FD, with automatic cleanup */

0 commit comments

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