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 19073Attachments:
/pgpatches/rmdirtext/plainDownload+39-0
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
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 19073Bruce 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
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 19073Bruce 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
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 19073Attachments:
/pgpatches/rmdirtext/plainDownload+28-15
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
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