On Thu, Mar 06, 2008 at 10:19:37AM +0000, Peter Stephenson wrote:
> If we decide a warning message is still appropriate, given that we've
> detected exactly what's going on it should probably be more specific
> than just a vague "failed to write history".
Yes, we should definitely be giving the user better information.
I've fixed several things:
1. Tell the user when we're refusing to write out the history file so
that the ownership won't change. Try to differentiate if we're just
skipping the rewrite or if no new history data was written.
2. Allow root to write out the file even if it doesn't own it, as long
as zsh knows how to chown a file handle.
3. Avoid the generic failure message that confuses the issue (by setting
"err = 0").
4. (bonus) Output the right error if we can't open the file for writing,
not an error about the file-handle being invalid. (This was due to the
use of fdopen(open(...), ...), which loses the errno from open().)
..wayne..
--- Src/hist.c 31 Dec 2007 23:14:17 -0000 1.70
+++ Src/hist.c 6 Mar 2008 16:43:05 -0000
@@ -2192,13 +2192,13 @@ savehistfile(char *fn, int err, int writ
}
errno = 0;
if (writeflags & HFILE_APPEND) {
+ int fd = open(unmeta(fn), O_CREAT | O_WRONLY | O_APPEND | O_NOCTTY,
0600);
tmpfile = NULL;
- out = fdopen(open(unmeta(fn),
- O_CREAT | O_WRONLY | O_APPEND | O_NOCTTY, 0600), "a");
+ out = fd >= 0 ? fdopen(fd, "a") : NULL;
} else if (!isset(HISTSAVEBYCOPY)) {
+ int fd = open(unmeta(fn), O_CREAT | O_WRONLY | O_TRUNC | O_NOCTTY,
0600);
tmpfile = NULL;
- out = fdopen(open(unmeta(fn),
- O_CREAT | O_WRONLY | O_TRUNC | O_NOCTTY, 0600), "w");
+ out = fd >= 0 ? fdopen(fd, "w") : NULL;
} else {
tmpfile = bicat(unmeta(fn), ".new");
if (unlink(tmpfile) < 0 && errno != ENOENT)
@@ -2206,13 +2206,27 @@ savehistfile(char *fn, int err, int writ
else {
struct stat sb;
int old_exists = stat(unmeta(fn), &sb) == 0;
+ uid_t euid = geteuid();
- if (old_exists && sb.st_uid != geteuid()) {
+ if (old_exists
+#if defined HAVE_FCHMOD && defined HAVE_FCHOWN
+ && euid
+#endif
+ && sb.st_uid != euid) {
free(tmpfile);
- tmpfile = NULL; /* Avoid an error about HISTFILE.new */
+ if (err) {
+ if (isset(APPENDHISTORY) || isset(INCAPPENDHISTORY)
+ || isset(SHAREHISTORY))
+ zerr("writing %s would change its ownership -- skipped
size trimming", fn);
+ else
+ zerr("writing %s would change its ownership -- history
not saved", fn);
+ err = 0; /* Don't report a generic error below. */
+ }
out = NULL;
- } else
- out = fdopen(open(tmpfile, O_CREAT | O_WRONLY | O_EXCL, 0600),
"w");
+ } else {
+ int fd = open(tmpfile, O_CREAT | O_WRONLY | O_EXCL, 0600);
+ out = fd >= 0 ? fdopen(fd, "w") : NULL;
+ }
#ifdef HAVE_FCHMOD
if (old_exists && out) {
