[PATCHES] Errno checks for rmtree()
The following patch adds appropriate error messages based on the errno
returned from rmtree() failures.
The original code came in this commit:
revision 1.13
date: 2004/08/01 06:19:26; author: momjian; state: Exp; lines: +173
-7
Add docs for initdb --auth.
which obviouisly had an inaccurate description. I am not even sure
where the rmtree() code came from, but I think it was Andrew Dunstan.
Anyway, it never had errno checks with messages, so we didn't strip it
out; rather it was never there. The original code did a
system(rmdir) call so this code was obviously superior.
Anyway, I will apply the attached patch to CVS HEAD and 8.0.X in a day
or so.
--
Bruce Momjian| http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup.| Newtown Square, Pennsylvania 19073
Index: src/port/dirmod.c
===
RCS file: /cvsroot/pgsql/src/port/dirmod.c,v
retrieving revision 1.34
diff -c -c -r1.34 dirmod.c
*** src/port/dirmod.c 31 Dec 2004 22:03:53 - 1.34
--- src/port/dirmod.c 12 Feb 2005 22:25:58 -
***
*** 350,355
--- 350,356
return filenames;
}
+
/*
*fnames_cleanup
*
***
*** 366,371
--- 367,407
pfree(filenames);
}
+
+ /*
+ *rmtree_errno
+ *
+ *report rmtree errno failure messages
+ */
+ static void
+ rmtree_errno(char *filepath)
+ {
+ if (errno == EACCES)
+ #ifndef FRONTEND
+ elog(LOG, no permission to remove \%s\, filepath);
+ #else
+ fprintf(stderr, no permission to remove \%s\, filepath);
+ #endif
+ else if (errno == EBUSY)
+ #ifndef FRONTEND
+ elog(LOG, can not remove \%s\ because it is in use,
filepath);
+ #else
+ fprintf(stderr, can not remove \%s\ because it is in use,
filepath);
+ #endif
+ else if (errno == ENOENT)
+ #ifndef FRONTEND
+ elog(LOG, \%s\ disappeared during directory removal,
filepath);
+ #else
+ fprintf(stderr, \%s\ disappeared during directory removal,
filepath);
+ #endif
+ else
+ #ifndef FRONTEND
+ elog(LOG, can not remove \%s\, filepath);
+ #else
+ fprintf(stderr, can not remove \%s\, filepath);
+ #endif
+ }
+
/*
*rmtree
*
***
*** 399,404
--- 435,441
if (stat(filepath, statbuf) != 0)
{
+ rmtree_errno(*filename);
fnames_cleanup(filenames);
return false;
}
***
*** 416,421
--- 453,459
{
if (unlink(filepath) != 0)
{
+ rmtree_errno(*filename);
fnames_cleanup(filenames);
return false;
}
***
*** 426,431
--- 464,470
{
if (rmdir(path) != 0)
{
+ rmtree_errno(*filename);
fnames_cleanup(filenames);
return false;
}
---(end of broadcast)---
TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] Errno checks for rmtree()
Bruce Momjian pgman@candle.pha.pa.us writes:
+ static void
+ rmtree_errno(char *filepath)
+ {
+ if (errno == EACCES)
+ #ifndef FRONTEND
+ elog(LOG, no permission to remove \%s\, filepath);
+ #else
+ fprintf(stderr, no permission to remove \%s\, filepath);
+ #endif
+ else if (errno == EBUSY)
[ etc ]
This seems awfully bogus: it's incomplete and not in the style of our
other error messages. Why not just one case:
#ifndef FRONTEND
elog(LOG, could not remove \%s\: %m, filepath);
#else
fprintf(stderr, could not remove \%s\: %s\n, filepath,
strerror(errno));
#endif
Also, I'm unconvinced that LOG is the appropriate error level;
WARNING would probably be better.
regards, tom lane
---(end of broadcast)---
TIP 2: you can get off all lists at once with the unregister command
(send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [PATCHES] Errno checks for rmtree()
Tom Lane wrote:
Bruce Momjian pgman@candle.pha.pa.us writes:
+ static void
+ rmtree_errno(char *filepath)
+ {
+ if (errno == EACCES)
+ #ifndef FRONTEND
+ elog(LOG, no permission to remove \%s\, filepath);
+ #else
+ fprintf(stderr, no permission to remove \%s\, filepath);
+ #endif
+ else if (errno == EBUSY)
[ etc ]
This seems awfully bogus: it's incomplete and not in the style of our
other error messages. Why not just one case:
#ifndef FRONTEND
elog(LOG, could not remove \%s\: %m, filepath);
#else
fprintf(stderr, could not remove \%s\: %s\n, filepath,
strerror(errno));
#endif
OK, so you just wanted _an_ error string to be printed on failure rather
than the reasons.
Also, I'm unconvinced that LOG is the appropriate error level;
WARNING would probably be better.
Yea, good point.
--
Bruce Momjian| http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup.| Newtown Square, Pennsylvania 19073
---(end of broadcast)---
TIP 9: the planner will ignore your desire to choose an index scan if your
joining column's datatypes do not match
Re: [PATCHES] Errno checks for rmtree()
Tom Lane wrote: Bruce Momjian pgman@candle.pha.pa.us writes: OK, so you just wanted _an_ error string to be printed on failure rather than the reasons. Well, what we want to know is the particular file that couldn't be deleted and the errno code. This is all can't happen stuff and so it just has to be a message that is useful for debugging; I don't see that it has to be amazingly user-friendly. Wow, you want to print out the raw errno number. We only do that in one or two places in our code that I could fine. -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] Errno checks for rmtree()
Bruce Momjian pgman@candle.pha.pa.us writes: OK, so you just wanted _an_ error string to be printed on failure rather than the reasons. Well, what we want to know is the particular file that couldn't be deleted and the errno code. This is all can't happen stuff and so it just has to be a message that is useful for debugging; I don't see that it has to be amazingly user-friendly. regards, tom lane ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] Errno checks for rmtree()
Bruce Momjian pgman@candle.pha.pa.us writes: Wow, you want to print out the raw errno number. No, I didn't say that. I wanted the strerror result, and that's what the code I suggested does. regards, tom lane ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] Errno checks for rmtree()
Tom Lane wrote:
Bruce Momjian pgman@candle.pha.pa.us writes:
Wow, you want to print out the raw errno number.
No, I didn't say that. I wanted the strerror result, and that's what
the code I suggested does.
OK, new version.
--
Bruce Momjian| http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup.| Newtown Square, Pennsylvania 19073
Index: src/port/dirmod.c
===
RCS file: /cvsroot/pgsql/src/port/dirmod.c,v
retrieving revision 1.34
diff -c -c -r1.34 dirmod.c
*** src/port/dirmod.c 31 Dec 2004 22:03:53 - 1.34
--- src/port/dirmod.c 13 Feb 2005 01:51:35 -
***
*** 350,355
--- 350,356
return filenames;
}
+
/*
*fnames_cleanup
*
***
*** 366,371
--- 367,373
pfree(filenames);
}
+
/*
*rmtree
*
***
*** 398,413
snprintf(filepath, MAXPGPATH, %s/%s, path, *filename);
if (stat(filepath, statbuf) != 0)
! {
! fnames_cleanup(filenames);
! return false;
! }
if (S_ISDIR(statbuf.st_mode))
{
/* call ourselves recursively for a directory */
if (!rmtree(filepath, true))
{
fnames_cleanup(filenames);
return false;
}
--- 400,413
snprintf(filepath, MAXPGPATH, %s/%s, path, *filename);
if (stat(filepath, statbuf) != 0)
! goto report_and_fail;
if (S_ISDIR(statbuf.st_mode))
{
/* call ourselves recursively for a directory */
if (!rmtree(filepath, true))
{
+ /* we already reported the error */
fnames_cleanup(filenames);
return false;
}
***
*** 415,436
else
{
if (unlink(filepath) != 0)
! {
! fnames_cleanup(filenames);
! return false;
! }
}
}
if (rmtopdir)
{
if (rmdir(path) != 0)
! {
! fnames_cleanup(filenames);
! return false;
! }
}
fnames_cleanup(filenames);
return true;
}
--- 415,440
else
{
if (unlink(filepath) != 0)
! goto report_and_fail;
}
}
if (rmtopdir)
{
if (rmdir(path) != 0)
! goto report_and_fail;
}
fnames_cleanup(filenames);
return true;
+
+ report_and_fail:
+
+ #ifndef FRONTEND
+ elog(WARNING, can not remove \%s\: %s, filepath, strerror(errno));
+ #else
+ fprintf(stderr, can not remove \%s\: %s\n, filepath,
strerror(errno));
+ #endif
+ fnames_cleanup(filenames);
+ return false;
}
---(end of broadcast)---
TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] Errno checks for rmtree()
Bruce Momjian pgman@candle.pha.pa.us writes: + elog(WARNING, can not remove \%s\: %s, filepath, strerror(errno)); could not remove, please; read the message style guidelines. Also, what's wrong with using %m here? Otherwise it looks good. regards, tom lane ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] Errno checks for rmtree()
Tom Lane wrote:
Bruce Momjian pgman@candle.pha.pa.us writes:
+ elog(WARNING, can not remove \%s\: %s, filepath, strerror(errno));
could not remove, please; read the message style guidelines.
Also, what's wrong with using %m here?
Otherwise it looks good.
OK, new version attached.
--
Bruce Momjian| http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup.| Newtown Square, Pennsylvania 19073
Index: src/port/dirmod.c
===
RCS file: /cvsroot/pgsql/src/port/dirmod.c,v
retrieving revision 1.34
diff -c -c -r1.34 dirmod.c
*** src/port/dirmod.c 31 Dec 2004 22:03:53 - 1.34
--- src/port/dirmod.c 13 Feb 2005 02:23:22 -
***
*** 350,355
--- 350,356
return filenames;
}
+
/*
*fnames_cleanup
*
***
*** 366,371
--- 367,373
pfree(filenames);
}
+
/*
*rmtree
*
***
*** 398,413
snprintf(filepath, MAXPGPATH, %s/%s, path, *filename);
if (stat(filepath, statbuf) != 0)
! {
! fnames_cleanup(filenames);
! return false;
! }
if (S_ISDIR(statbuf.st_mode))
{
/* call ourselves recursively for a directory */
if (!rmtree(filepath, true))
{
fnames_cleanup(filenames);
return false;
}
--- 400,413
snprintf(filepath, MAXPGPATH, %s/%s, path, *filename);
if (stat(filepath, statbuf) != 0)
! goto report_and_fail;
if (S_ISDIR(statbuf.st_mode))
{
/* call ourselves recursively for a directory */
if (!rmtree(filepath, true))
{
+ /* we already reported the error */
fnames_cleanup(filenames);
return false;
}
***
*** 415,436
else
{
if (unlink(filepath) != 0)
! {
! fnames_cleanup(filenames);
! return false;
! }
}
}
if (rmtopdir)
{
if (rmdir(path) != 0)
! {
! fnames_cleanup(filenames);
! return false;
! }
}
fnames_cleanup(filenames);
return true;
}
--- 415,440
else
{
if (unlink(filepath) != 0)
! goto report_and_fail;
}
}
if (rmtopdir)
{
if (rmdir(path) != 0)
! goto report_and_fail;
}
fnames_cleanup(filenames);
return true;
+
+ report_and_fail:
+
+ #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
+ fnames_cleanup(filenames);
+ return false;
}
---(end of broadcast)---
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to [EMAIL PROTECTED] so that your
message can get through to the mailing list cleanly
