Zsh Mailing List Archive
Messages sorted by:
Reverse Date,
Date,
Thread,
Author
Re: No fsync on history file? I lost my history
- X-seq: zsh-users 23676
- From: Mikael Magnusson <mikachu@xxxxxxxxx>
- To: lilydjwg <lilydjwg@xxxxxxxxx>
- Subject: Re: No fsync on history file? I lost my history
- Date: Sun, 23 Sep 2018 20:02:44 +0200
- Cc: Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>, Zsh Users <zsh-users@xxxxxxx>
- Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=yRr5RBq1mjmqmKFidcQWSAxPcatBhzd/FfK5KTHFZq0=; b=BoluSWhlx2R8E/4C33OQoFUjv5+gPOvkzi8l3fBG6kOkEnHUzMVwH1yLrOEtm0wUIk iHgvSyvPNfqaExBdn5nbBFDUE3VdTgpbOeIQAdzGU9ItqJGEFfPCIf/sE/nabfO/4fec q0dDwZ42Xv/5ZZFYQbnOo96DaHK4r+LfBso0nFG05tFbA+Cc5BdWydMTes5PTCpoeJ1a 6D3J+zGIi21o0FQO48adb/wRu4/EBPgcbLmwAEYJ1zDFlKGQjM2oAGU+px5hsV0o6GoF trhafG44+dceAt2G15co5WV6BRo4UAyANrhv1l3IczIUDm04sAOOfEt7QzE0oO2ofhrb kb6w==
- In-reply-to: <20180923152546.GA6201@lilyforest.localdomain>
- List-help: <mailto:zsh-users-help@zsh.org>
- List-id: Zsh Users List <zsh-users.zsh.org>
- List-post: <mailto:zsh-users@zsh.org>
- List-unsubscribe: <mailto:zsh-users-unsubscribe@zsh.org>
- Mailing-list: contact zsh-users-help@xxxxxxx; run by ezmlm
- References: <20180923085246.GA19251@lilyforest.localdomain> <1537709747.103981.1517680056.72C7A43E@webmail.messagingengine.com> <20180923142255.GA4931@lilyforest.localdomain> <1537714011.118073.1517716184.0B2E8824@webmail.messagingengine.com> <20180923152546.GA6201@lilyforest.localdomain>
On Sun, Sep 23, 2018 at 5:25 PM, lilydjwg <lilydjwg@xxxxxxxxx> wrote:
> I'm sending an updated patch.
>
> On Sun, Sep 23, 2018 at 02:46:51PM +0000, Daniel Shahaf wrote:
>> fsync() is in POSIX. I assume we can just call it, but if somebody complains
>> we'll need to use an HAVE_FSYNC guard.
>
> I don't know how to add a HAVE_FSYNC macro to the build system, sorry.
>
>> > +++ b/Src/hist.c
>> > @@ -2933,6 +2933,9 @@ savehistfile(char *fn, int err, int writeflags)
>> > lasthist.text = ztrdup(start);
>> > }
>> > }
>> > + fflush(out); /* need to flush before fsync */
>>
>> Isn't the fflush() on line 2927 sufficient? (Even if it isn't, I would have
>> expected a ret>=0 guard around this call.)
>
> It should call write(2) to write out the buffered data. Then the kernel
> can fsync the data to disk. A guard has been added.
>
>> > + if (fsync(fileno(out)) < 0 && ret >= 0)
>> > + ret = -1;
>>
>> fileno() can return -1.
>
> It shouldn't matter, fsync will return EBADF for -1. Other parts of the
> code don't check for this either, and I can't think a case when fileno
> would fail after so many successful I/O operations on it (corrupted memory?)
>
>> Shouldn't the ret>=0 check happen before the calls to fileno() and fsync()?
>
> Yes, I've changed that.
>
> --
> Best regards,
> lilydjwg
>
> From 3c6c07733f12176c737d1f610f0dceafd07437df Mon Sep 17 00:00:00 2001
> From: lilydjwg <lilydjwg@xxxxxxxxx>
> Date: Sun, 23 Sep 2018 22:12:56 +0800
> Subject: [PATCH] Call fsync after writing out new histfile
>
> to ensure the data is on disk before the rename in case of a system crash.
> ---
> Src/hist.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/Src/hist.c b/Src/hist.c
> index dbdc1e4e5..d3370252d 100644
> --- a/Src/hist.c
> +++ b/Src/hist.c
> @@ -2933,6 +2933,10 @@ savehistfile(char *fn, int err, int writeflags)
> lasthist.text = ztrdup(start);
> }
> }
> + if (ret >= 0)
> + ret = fflush(out); /* need to flush before fsync */
> + if (ret >= 0 && fsync(fileno(out)) < 0)
> + ret = -1;
> if (fclose(out) < 0 && ret >= 0)
> ret = -1;
> if (ret >= 0) {
Please only do this for the tmpfile case, or interactive usage is
going to be unbearable under load with incappendhistory.
--
Mikael Magnusson
Messages sorted by:
Reverse Date,
Date,
Thread,
Author