On Mon, Dec 9, 2013 at 11:27:28AM -0500, Robert Haas wrote:
> On Thu, Dec 5, 2013 at 6:15 PM, Tom Lane <[email protected]> wrote:
> > But the other usages seem to be in assorted utilities, which
> > will need to do it right for themselves. initdb.c's walkdir() seems to
> > have it right and might be a reasonable model to follow. Or maybe we
> > should invent a frontend-friendly version of ReadDir() rather than
> > duplicating all the error checking code in ten-and-counting places?
>
> If there's enough uniformity in all of those places to make that
> feasible, it certainly seems wise to do it that way. I don't know if
> that's the case, though - e.g. maybe some callers want to exit and
> others do not. pg_resetxlog wants to exit; pg_archivecleanup and
> pg_standby most likely want to print an error and carry on.
I have developed the attached patch which fixes all cases where
readdir() wasn't checking for errno, and cleaned up the syntax in other
cases to be consistent.
While I am not a fan of backpatching, the fact we are ignoring errors in
some critical cases seems the non-cosmetic parts should be backpatched.
--
Bruce Momjian <[email protected]> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
diff --git a/contrib/pg_archivecleanup/pg_archivecleanup.c b/contrib/pg_archivecleanup/pg_archivecleanup.c
new file mode 100644
index 7b5484b..fbc3e5a
*** a/contrib/pg_archivecleanup/pg_archivecleanup.c
--- b/contrib/pg_archivecleanup/pg_archivecleanup.c
*************** CleanupPriorWALFiles(void)
*** 106,112 ****
if ((xldir = opendir(archiveLocation)) != NULL)
{
! while ((xlde = readdir(xldir)) != NULL)
{
strncpy(walfile, xlde->d_name, MAXPGPATH);
TrimExtension(walfile, additional_ext);
--- 106,112 ----
if ((xldir = opendir(archiveLocation)) != NULL)
{
! while (errno = 0, (xlde = readdir(xldir)) != NULL)
{
strncpy(walfile, xlde->d_name, MAXPGPATH);
TrimExtension(walfile, additional_ext);
*************** CleanupPriorWALFiles(void)
*** 164,169 ****
--- 164,172 ----
}
}
}
+ if (errno)
+ fprintf(stderr, "%s: could not read archive location \"%s\": %s\n",
+ progname, archiveLocation, strerror(errno));
closedir(xldir);
}
else
diff --git a/contrib/pg_standby/pg_standby.c b/contrib/pg_standby/pg_standby.c
new file mode 100644
index 8ddd486..be4d31f
*** a/contrib/pg_standby/pg_standby.c
--- b/contrib/pg_standby/pg_standby.c
*************** CustomizableCleanupPriorWALFiles(void)
*** 245,251 ****
*/
if ((xldir = opendir(archiveLocation)) != NULL)
{
! while ((xlde = readdir(xldir)) != NULL)
{
/*
* We ignore the timeline part of the XLOG segment identifiers
--- 245,251 ----
*/
if ((xldir = opendir(archiveLocation)) != NULL)
{
! while (errno = 0, (xlde = readdir(xldir)) != NULL)
{
/*
* We ignore the timeline part of the XLOG segment identifiers
*************** CustomizableCleanupPriorWALFiles(void)
*** 283,288 ****
--- 283,291 ----
}
}
}
+ if (errno)
+ fprintf(stderr, "%s: could not read archive location \"%s\": %s\n",
+ progname, archiveLocation, strerror(errno));
if (debug)
fprintf(stderr, "\n");
}
diff --git a/src/bin/pg_basebackup/pg_receivexlog.c b/src/bin/pg_basebackup/pg_receivexlog.c
new file mode 100644
index 2478789..e252405
*** a/src/bin/pg_basebackup/pg_receivexlog.c
--- b/src/bin/pg_basebackup/pg_receivexlog.c
*************** FindStreamingStart(uint32 *tli)
*** 139,145 ****
disconnect_and_exit(1);
}
! while ((dirent = readdir(dir)) != NULL)
{
uint32 tli;
XLogSegNo segno;
--- 139,145 ----
disconnect_and_exit(1);
}
! while (errno = 0, (dirent = readdir(dir)) != NULL)
{
uint32 tli;
XLogSegNo segno;
*************** FindStreamingStart(uint32 *tli)
*** 209,214 ****
--- 209,221 ----
}
}
+ if (errno)
+ {
+ fprintf(stderr, _("%s: could not read directory \"%s\": %s\n"),
+ progname, basedir, strerror(errno));
+ disconnect_and_exit(1);
+ }
+
closedir(dir);
if (high_segno > 0)
diff --git a/src/bin/pg_dump/pg_backup_directory.c b/src/bin/pg_dump/pg_backup_directory.c
new file mode 100644
index 1bed8a9..c24f7e3
*** a/src/bin/pg_dump/pg_backup_directory.c
--- b/src/bin/pg_dump/pg_backup_directory.c
*************** InitArchiveFmt_Directory(ArchiveHandle *
*** 177,183 ****
struct dirent *d;
is_empty = true;
! while ((d = readdir(dir)))
{
if (strcmp(d->d_name, ".") != 0 && strcmp(d->d_name, "..") != 0)
{
--- 177,183 ----
struct dirent *d;
is_empty = true;
! while (errno = 0, (d = readdir(dir)))
{
if (strcmp(d->d_name, ".") != 0 && strcmp(d->d_name, "..") != 0)
{
*************** InitArchiveFmt_Directory(ArchiveHandle *
*** 185,190 ****
--- 185,193 ----
break;
}
}
+ if (errno)
+ exit_horribly(modulename, "could not read directory \"%s\": %s\n",
+ ctx->directory, strerror(errno));
closedir(dir);
}
}
diff --git a/src/bin/pg_resetxlog/pg_resetxlog.c b/src/bin/pg_resetxlog/pg_resetxlog.c
new file mode 100644
index 28a4f19..7bd8052
*** a/src/bin/pg_resetxlog/pg_resetxlog.c
--- b/src/bin/pg_resetxlog/pg_resetxlog.c
*************** FindEndOfXLOG(void)
*** 821,828 ****
exit(1);
}
! errno = 0;
! while ((xlde = readdir(xldir)) != NULL)
{
if (strlen(xlde->d_name) == 24 &&
strspn(xlde->d_name, "0123456789ABCDEF") == 24)
--- 821,827 ----
exit(1);
}
! while (errno = 0, (xlde = readdir(xldir)) != NULL)
{
if (strlen(xlde->d_name) == 24 &&
strspn(xlde->d_name, "0123456789ABCDEF") == 24)
*************** FindEndOfXLOG(void)
*** 844,850 ****
if (segno > newXlogSegNo)
newXlogSegNo = segno;
}
- errno = 0;
}
#ifdef WIN32
--- 843,848 ----
*************** KillExistingXLOG(void)
*** 892,899 ****
exit(1);
}
! errno = 0;
! while ((xlde = readdir(xldir)) != NULL)
{
if (strlen(xlde->d_name) == 24 &&
strspn(xlde->d_name, "0123456789ABCDEF") == 24)
--- 890,896 ----
exit(1);
}
! while (errno = 0, (xlde = readdir(xldir)) != NULL)
{
if (strlen(xlde->d_name) == 24 &&
strspn(xlde->d_name, "0123456789ABCDEF") == 24)
*************** KillExistingXLOG(void)
*** 906,912 ****
exit(1);
}
}
- errno = 0;
}
#ifdef WIN32
--- 903,908 ----
*************** KillExistingArchiveStatus(void)
*** 948,955 ****
exit(1);
}
! errno = 0;
! while ((xlde = readdir(xldir)) != NULL)
{
if (strspn(xlde->d_name, "0123456789ABCDEF") == 24 &&
(strcmp(xlde->d_name + 24, ".ready") == 0 ||
--- 944,950 ----
exit(1);
}
! while (errno = 0, (xlde = readdir(xldir)) != NULL)
{
if (strspn(xlde->d_name, "0123456789ABCDEF") == 24 &&
(strcmp(xlde->d_name + 24, ".ready") == 0 ||
*************** KillExistingArchiveStatus(void)
*** 963,969 ****
exit(1);
}
}
- errno = 0;
}
#ifdef WIN32
--- 958,963 ----
diff --git a/src/common/pgfnames.c b/src/common/pgfnames.c
new file mode 100644
index 9a68163..0d852b1
*** a/src/common/pgfnames.c
--- b/src/common/pgfnames.c
*************** pgfnames(const char *path)
*** 50,57 ****
filenames = (char **) palloc(fnsize * sizeof(char *));
! errno = 0;
! while ((file = readdir(dir)) != NULL)
{
if (strcmp(file->d_name, ".") != 0 && strcmp(file->d_name, "..") != 0)
{
--- 50,56 ----
filenames = (char **) palloc(fnsize * sizeof(char *));
! while (errno = 0, (file = readdir(dir)) != NULL)
{
if (strcmp(file->d_name, ".") != 0 && strcmp(file->d_name, "..") != 0)
{
*************** pgfnames(const char *path)
*** 63,69 ****
}
filenames[numnames++] = pstrdup(file->d_name);
}
- errno = 0;
}
#ifdef WIN32
--- 62,67 ----
diff --git a/src/port/pgcheckdir.c b/src/port/pgcheckdir.c
new file mode 100644
index fc97f8c..6454525
*** a/src/port/pgcheckdir.c
--- b/src/port/pgcheckdir.c
*************** pg_check_dir(const char *dir)
*** 33,46 ****
struct dirent *file;
bool dot_found = false;
- errno = 0;
-
chkdir = opendir(dir);
-
if (chkdir == NULL)
return (errno == ENOENT) ? 0 : -1;
! while ((file = readdir(chkdir)) != NULL)
{
if (strcmp(".", file->d_name) == 0 ||
strcmp("..", file->d_name) == 0)
--- 33,43 ----
struct dirent *file;
bool dot_found = false;
chkdir = opendir(dir);
if (chkdir == NULL)
return (errno == ENOENT) ? 0 : -1;
! while (errno = 0, (file = readdir(chkdir)) != NULL)
{
if (strcmp(".", file->d_name) == 0 ||
strcmp("..", file->d_name) == 0)
*************** pg_check_dir(const char *dir)
*** 76,84 ****
errno = 0;
#endif
! closedir(chkdir);
!
! if (errno != 0)
result = -1; /* some kind of I/O error? */
/* We report on dot-files if we _only_ find dot files */
--- 73,79 ----
errno = 0;
#endif
! if (errno || closedir(chkdir) == -1)
result = -1; /* some kind of I/O error? */
/* We report on dot-files if we _only_ find dot files */
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
