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.

diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 9596bf4808158fd7a2665a6d508b1e8bbceb13e1..3869b0419669d37843cf3f151f34e51c760c9f5b 100644 (file)
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -13,7 +13,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/commands/dbcommands.c,v 1.204.2.2 2008/04/18 06:48:50 heikki Exp $
+ *   $PostgreSQL: pgsql/src/backend/commands/dbcommands.c,v 1.204.2.3 2008/04/18 17:05:53 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1320,7 +1320,7 @@ remove_dbtablespaces(Oid db_id)
 
        if (!rmtree(dstpath, true))
            ereport(WARNING,
-                   (errmsg("could not remove database directory \"%s\"",
+                   (errmsg("some useless files may be left behind in old database directory \"%s\"",
                            dstpath)));
 
        /* Record the filesystem change in XLOG */
@@ -1489,7 +1489,7 @@ dbase_redo(XLogRecPtr lsn, XLogRecord *record)
        {
            if (!rmtree(dst_path, true))
                ereport(WARNING,
-                       (errmsg("could not remove database directory \"%s\"",
+                       (errmsg("some useless files may be left behind in old database directory \"%s\"",
                                dst_path)));
        }
 
@@ -1528,7 +1528,7 @@ dbase_redo(XLogRecPtr lsn, XLogRecord *record)
        /* And remove the physical files */
        if (!rmtree(dst_path, true))
            ereport(WARNING,
-                   (errmsg("could not remove database directory \"%s\"",
+                   (errmsg("some useless files may be left behind in old database directory \"%s\"",
                            dst_path)));
    }
    else

diff --git a/src/include/port.h b/src/include/port.h
index f6ccfaecba7a3fca405821277d78c5f310b7b5ae..97828a775ab44e71b7b42195798d28cf2306e1ab 100644 (file)
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -6,7 +6,7 @@
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/port.h,v 1.116.2.4 2008/04/16 14:21:22 adunstan Exp $
+ * $PostgreSQL: pgsql/src/include/port.h,v 1.116.2.5 2008/04/18 17:05:53 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -47,7 +47,7 @@ extern bool get_home_path(char *ret_path);
 extern void get_parent_directory(char *path);
 
 /* port/dirmod.c */
-extern char **pgfnames(char *path);
+extern char **pgfnames(const char *path);
 extern void pgfnames_cleanup(char **filenames);
 
 /*
@@ -278,7 +278,7 @@ extern int  pgsymlink(const char *oldpath, const char *newpath);
 
 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

diff --git a/src/port/dirmod.c b/src/port/dirmod.c
index 0138be89d4ec488ba41bef292ebff5ca1f182c5d..70c4639a1d4ffa90de644306c0ddca1daca50a63 100644 (file)
--- a/src/port/dirmod.c
+++ b/src/port/dirmod.c
@@ -10,7 +10,7 @@
  * Win32 (NT, Win2k, XP).  replace() doesn't work on Win95/98/Me.
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/port/dirmod.c,v 1.51.2.3 2008/04/18 06:48:50 heikki Exp $
+ *   $PostgreSQL: pgsql/src/port/dirmod.c,v 1.51.2.4 2008/04/18 17:05:53 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
  * function.
  */
-char     **
-pgfnames(char *path)
+char **
+pgfnames(const char *path)
 {
    DIR        *dir;
    struct dirent *file;
@@ -380,12 +380,15 @@ pgfnames_cleanup(char **filenames)
  * Assumes path points to a valid directory.
  * Deletes everything under path.
  * 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
-rmtree(char *path, bool rmtopdir)
+rmtree(const char *path, bool rmtopdir)
 {
+   bool        result = true;
    char        pathbuf[MAXPGPATH];
-   char       *filepath;
    char      **filenames;
    char      **filename;
    struct stat statbuf;
@@ -400,11 +403,9 @@ rmtree(char *path, bool rmtopdir)
        return false;
 
    /* now we have the names we can start removing things */
-   filepath = pathbuf;
-
    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
@@ -417,54 +418,68 @@ rmtree(char *path, bool rmtopdir)
         * requests, but because that's asynchronous, it's not guaranteed
         * that the bgwriter receives the message in time.
         */
-       if (lstat(filepath, &statbuf) != 0)
+       if (lstat(pathbuf, &statbuf) != 0)
        {
            if (errno != ENOENT)
-               goto report_and_fail;
-           else
-               continue;
+           {
+#ifndef FRONTEND
+               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))
        {
            /* call ourselves recursively for a directory */
-           if (!rmtree(filepath, true))
+           if (!rmtree(pathbuf, true))
            {
                /* we already reported the error */
-               pgfnames_cleanup(filenames);
-               return false;
+               result = false;
            }
        }
        else
        {
-           if (unlink(filepath) != 0)
+           if (unlink(pathbuf) != 0)
            {
                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)
    {
-       filepath = path;
-       if (rmdir(filepath) != 0)
-           goto report_and_fail;
-   }
-
-   pgfnames_cleanup(filenames);
-   return true;
-
-report_and_fail:
-
+       if (rmdir(path) != 0)
+       {
 #ifndef FRONTEND
-   elog(WARNING, "could not remove file or directory \"%s\": %m", filepath);
+           elog(WARNING, "could not remove file or directory \"%s\": %m",
+                path);
 #else
-   fprintf(stderr, _("could not remove file or directory \"%s\": %s\n"),
-           filepath, strerror(errno));
+           fprintf(stderr, _("could not remove file or directory \"%s\": %s\n"),
+                   path, strerror(errno));
 #endif
+           result = false;
+       }
+   }
+
    pgfnames_cleanup(filenames);
-   return false;
+
+   return result;
 }