Hi all, On the recent following thread problems around the use of AllocateDir() have been diagnosed with its use in the backend code: https://www.postgresql.org/message-id/20171127093107.1473.70...@wrigleys.postgresql.org
I had a close look at all the callers of AllocateDir() and noticed a couple of unwelcome things (Tom noticed some of those in the thread mentioned above, I found others): - Some elog() instead of ereport(), which is bad for the error code and translation. - Some use ereport(LOG), and could take advantage of ReadDirExtended, which could get exposed instead of being contained in fd.c. Those elog(LOG) can be removed when there is no further operation in the routine where the folder read is happening. - perform_base_backup() makes the mistake of not saving errno before CheckXLogRemoved() when AllocateDir returns NULL, which can lead to an incorrect error message. - restoreTwoPhaseData() calls ReadDir() with LWLockAcquire() in-between, which can lead to an incorrect errno used. - Some code paths can simply rely on the error generated by ReadDir(), so some code is useless. - Some code paths use non-generic error messages, like RemoveOldXlogFiles(). Here more consistent error string would I think make sense. Some issues are real bugs, like what's in perform_base_backup() and restoreTwoPhaseData(), and some incorrect error codes. Missing translation of some messages is also wrong IMO. Making error messages more consistent is nice and cosmetic. My monitoring of all those issues is leading me to the attached patch for HEAD. Thoughts? -- Michael
allocatedir-errands.patch
Description: Binary data
