Fix rmtree() so that it keeps going after failure to remove any individual

file; the idea is that we should clean up as much as we can, even if there's
some problem removing one file.  Make the error messages a bit less misleading,
too.  In passing, const-ify function arguments.
This commit is contained in:
Tom Lane 2008-04-18 17:05:45 +00:00
parent 9cb91f90c9
commit b8e5581d76
3 changed files with 53 additions and 38 deletions

View File

@ -13,7 +13,7 @@
* *
* *
* IDENTIFICATION * IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/commands/dbcommands.c,v 1.207 2008/04/18 06:48:38 heikki Exp $ * $PostgreSQL: pgsql/src/backend/commands/dbcommands.c,v 1.208 2008/04/18 17:05:45 tgl Exp $
* *
*------------------------------------------------------------------------- *-------------------------------------------------------------------------
*/ */
@ -1321,7 +1321,7 @@ remove_dbtablespaces(Oid db_id)
if (!rmtree(dstpath, true)) if (!rmtree(dstpath, true))
ereport(WARNING, ereport(WARNING,
(errmsg("could not remove database directory \"%s\"", (errmsg("some useless files may be left behind in old database directory \"%s\"",
dstpath))); dstpath)));
/* Record the filesystem change in XLOG */ /* Record the filesystem change in XLOG */
@ -1490,7 +1490,7 @@ dbase_redo(XLogRecPtr lsn, XLogRecord *record)
{ {
if (!rmtree(dst_path, true)) if (!rmtree(dst_path, true))
ereport(WARNING, ereport(WARNING,
(errmsg("could not remove database directory \"%s\"", (errmsg("some useless files may be left behind in old database directory \"%s\"",
dst_path))); dst_path)));
} }
@ -1529,7 +1529,7 @@ dbase_redo(XLogRecPtr lsn, XLogRecord *record)
/* And remove the physical files */ /* And remove the physical files */
if (!rmtree(dst_path, true)) if (!rmtree(dst_path, true))
ereport(WARNING, ereport(WARNING,
(errmsg("could not remove database directory \"%s\"", (errmsg("some useless files may be left behind in old database directory \"%s\"",
dst_path))); dst_path)));
} }
else else

View File

@ -6,7 +6,7 @@
* Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California * Portions Copyright (c) 1994, Regents of the University of California
* *
* $PostgreSQL: pgsql/src/include/port.h,v 1.121 2008/04/16 14:19:56 adunstan Exp $ * $PostgreSQL: pgsql/src/include/port.h,v 1.122 2008/04/18 17:05:45 tgl Exp $
* *
*------------------------------------------------------------------------- *-------------------------------------------------------------------------
*/ */
@ -48,7 +48,7 @@ extern bool get_home_path(char *ret_path);
extern void get_parent_directory(char *path); extern void get_parent_directory(char *path);
/* port/dirmod.c */ /* port/dirmod.c */
extern char **pgfnames(char *path); extern char **pgfnames(const char *path);
extern void pgfnames_cleanup(char **filenames); extern void pgfnames_cleanup(char **filenames);
/* /*
@ -279,7 +279,7 @@ extern int pgsymlink(const char *oldpath, const char *newpath);
extern void copydir(char *fromdir, char *todir, bool recurse); extern void copydir(char *fromdir, char *todir, bool recurse);
extern bool rmtree(char *path, bool rmtopdir); extern bool rmtree(const char *path, bool rmtopdir);
/* /*
* stat() is not guaranteed to set the st_size field on win32, so we * stat() is not guaranteed to set the st_size field on win32, so we

View File

@ -10,7 +10,7 @@
* Win32 (NT, Win2k, XP). replace() doesn't work on Win95/98/Me. * Win32 (NT, Win2k, XP). replace() doesn't work on Win95/98/Me.
* *
* IDENTIFICATION * IDENTIFICATION
* $PostgreSQL: pgsql/src/port/dirmod.c,v 1.54 2008/04/18 06:48:38 heikki Exp $ * $PostgreSQL: pgsql/src/port/dirmod.c,v 1.55 2008/04/18 17:05:45 tgl Exp $
* *
*------------------------------------------------------------------------- *-------------------------------------------------------------------------
*/ */
@ -291,8 +291,8 @@ pgsymlink(const char *oldpath, const char *newpath)
* must call pgfnames_cleanup later to free the memory allocated by this * must call pgfnames_cleanup later to free the memory allocated by this
* function. * function.
*/ */
char ** char **
pgfnames(char *path) pgfnames(const char *path)
{ {
DIR *dir; DIR *dir;
struct dirent *file; struct dirent *file;
@ -380,12 +380,15 @@ pgfnames_cleanup(char **filenames)
* Assumes path points to a valid directory. * Assumes path points to a valid directory.
* Deletes everything under path. * Deletes everything under path.
* If rmtopdir is true deletes the directory too. * If rmtopdir is true deletes the directory too.
* Returns true if successful, false if there was any problem.
* (The details of the problem are reported already, so caller
* doesn't really have to say anything more, but most do.)
*/ */
bool bool
rmtree(char *path, bool rmtopdir) rmtree(const char *path, bool rmtopdir)
{ {
bool result = true;
char pathbuf[MAXPGPATH]; char pathbuf[MAXPGPATH];
char *filepath;
char **filenames; char **filenames;
char **filename; char **filename;
struct stat statbuf; struct stat statbuf;
@ -400,11 +403,9 @@ rmtree(char *path, bool rmtopdir)
return false; return false;
/* now we have the names we can start removing things */ /* now we have the names we can start removing things */
filepath = pathbuf;
for (filename = filenames; *filename; filename++) for (filename = filenames; *filename; filename++)
{ {
snprintf(filepath, MAXPGPATH, "%s/%s", path, *filename); snprintf(pathbuf, MAXPGPATH, "%s/%s", path, *filename);
/* /*
* It's ok if the file is not there anymore; we were just about to * It's ok if the file is not there anymore; we were just about to
@ -417,54 +418,68 @@ rmtree(char *path, bool rmtopdir)
* requests, but because that's asynchronous, it's not guaranteed * requests, but because that's asynchronous, it's not guaranteed
* that the bgwriter receives the message in time. * that the bgwriter receives the message in time.
*/ */
if (lstat(filepath, &statbuf) != 0) if (lstat(pathbuf, &statbuf) != 0)
{ {
if (errno != ENOENT) if (errno != ENOENT)
goto report_and_fail; {
else #ifndef FRONTEND
continue; elog(WARNING, "could not stat file or directory \"%s\": %m",
pathbuf);
#else
fprintf(stderr, _("could not stat file or directory \"%s\": %s\n"),
pathbuf, strerror(errno));
#endif
result = false;
}
continue;
} }
if (S_ISDIR(statbuf.st_mode)) if (S_ISDIR(statbuf.st_mode))
{ {
/* call ourselves recursively for a directory */ /* call ourselves recursively for a directory */
if (!rmtree(filepath, true)) if (!rmtree(pathbuf, true))
{ {
/* we already reported the error */ /* we already reported the error */
pgfnames_cleanup(filenames); result = false;
return false;
} }
} }
else else
{ {
if (unlink(filepath) != 0) if (unlink(pathbuf) != 0)
{ {
if (errno != ENOENT) if (errno != ENOENT)
goto report_and_fail; {
#ifndef FRONTEND
elog(WARNING, "could not remove file or directory \"%s\": %m",
pathbuf);
#else
fprintf(stderr, _("could not remove file or directory \"%s\": %s\n"),
pathbuf, strerror(errno));
#endif
result = false;
}
} }
} }
} }
if (rmtopdir) if (rmtopdir)
{ {
filepath = path; if (rmdir(path) != 0)
if (rmdir(filepath) != 0) {
goto report_and_fail; #ifndef FRONTEND
elog(WARNING, "could not remove file or directory \"%s\": %m",
path);
#else
fprintf(stderr, _("could not remove file or directory \"%s\": %s\n"),
path, strerror(errno));
#endif
result = false;
}
} }
pgfnames_cleanup(filenames); pgfnames_cleanup(filenames);
return true;
report_and_fail: return result;
#ifndef FRONTEND
elog(WARNING, "could not remove file or directory \"%s\": %m", filepath);
#else
fprintf(stderr, _("could not remove file or directory \"%s\": %s\n"),
filepath, strerror(errno));
#endif
pgfnames_cleanup(filenames);
return false;
} }