Re: Bug#369305: zsh: failed to write history file /home/sven/.zsh-history: no such file or directory

[Peter Stephenson <pws@xxxxxxx>]

On Wed, 5 Mar 2008 14:41:58 -0800
Wayne Davison <wayned@xxxxxxxxxxxxxxxxxxxxx> wrote:
> On Wed, Mar 05, 2008 at 03:29:36PM -0500, Clint Adams wrote:
> > As Sven notes, any history added before exiting will make it into the
> > history file despite the error message.
> 
> The history being added is the appending going on due to SHARE_HISTORY.
> The error is referring to the inability to rewrite the history file on
> exit, and it is actually failing (because the write-new, then rename
> steps would cause the file to change ownership).  If incremental history
> updating was not happening, the failed rewrite would indeed not save
> anything new.  The "no such file or directory" error string is
> presumably a left-over errno that Peter noticed.

Yes, I've found where... at this point we've already handled the error in
the manner you say by simply not trying to write the history, so there's no
system error we haven't dealt with and we should reset errno.

We could fix both problems by not printing an error if there's no "errno",
as below.  That's OK in the specific circumstances, but probably not right
in all cases.  Could we, for example, pass down a flag when savehistfile()
is called from itself to do the rewriting and suppress the error message,
or make it more anodyne, in that case?

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".

I won't commit this until we've resolved the TODO, but you might want to
check it does remove the error message.

Index: Src/hist.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/hist.c,v
retrieving revision 1.70
diff -u -r1.70 hist.c
--- Src/hist.c	31 Dec 2007 23:14:17 -0000	1.70
+++ Src/hist.c	6 Mar 2008 10:16:08 -0000
@@ -2207,7 +2207,13 @@
 	    struct stat sb;
 	    int old_exists = stat(unmeta(fn), &sb) == 0;
 
+	    errno = 0;
 	    if (old_exists && sb.st_uid != geteuid()) {
+		/*
+		 * TODO: do we want an error message about changed ownership
+		 * here?  Do we want a lesser error message, or none,
+		 * if this call to savehistfile() was just a rewrite?
+		 */
 		free(tmpfile);
 		tmpfile = NULL; /* Avoid an error about HISTFILE.new */
 		out = NULL;
@@ -2302,7 +2308,7 @@
     } else
 	ret = -1;
 
-    if (ret < 0 && err) {
+    if (ret < 0 && err && errno) {
 	if (tmpfile) {
 	    zerr("failed to write history file %s.new: %e", fn, errno);
 	    free(tmpfile);

-- 
Peter Stephenson <pws@xxxxxxx>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070