From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: zsh-users-return-23676-ml=inbox.vuxu.org@zsh.org X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FROM,MAILING_LIST_MULTI,RCVD_IN_DNSWL_NONE autolearn=ham autolearn_force=no version=3.4.2 Received: from primenet.com.au (ns1.primenet.com.au [203.24.36.2]) by inbox.vuxu.org (OpenSMTPD) with ESMTP id 079a8b19 for ; Sun, 23 Sep 2018 18:03:08 +0000 (UTC) Received: (qmail 9157 invoked by alias); 23 Sep 2018 18:02:51 -0000 Mailing-List: contact zsh-users-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Users List List-Post: List-Help: List-Unsubscribe: X-Seq: 23676 Received: (qmail 29003 invoked by uid 1010); 23 Sep 2018 18:02:50 -0000 X-Qmail-Scanner-Diagnostics: from mail-qt1-f193.google.com by f.primenet.com.au (envelope-from , uid 7791) with qmail-scanner-2.11 (clamdscan: 0.99.2/21882. spamassassin: 3.4.1. Clear:RC:0(209.85.160.193):SA:0(-1.9/5.0):. Processed in 2.299385 secs); 23 Sep 2018 18:02:50 -0000 X-Envelope-From: mikachu@gmail.com X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | 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== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=yRr5RBq1mjmqmKFidcQWSAxPcatBhzd/FfK5KTHFZq0=; b=LYvZaTnFphyIUbwpBjk6AoXYpmNOtE54F1U3ekcMDxb3pr5OOClrRwPen/z9JXYJWr 5vp+XuiGnqNUYEjqzejDt05WizdEJBrpGcMfSsu42coli7Q1vje2vUj+DrHtLx+XIdj+ ma5TkObPlK4Fpjz63XCH7VXlbeNUDQL2etP9BPhx0BKRbjExOf2hoz1hD7e3TiQUXQfQ pkBPfwBLH9JLI1a863rFNsS9vUvTE0QlIh8ZEUXVkadImAdjyNpnSPYAcDThwXfsyjT1 HBdcpXXhs9hA+x+OMmtUltunzggfn01KYH1riHmZqpCt0JMUKXeGLq17HJISHxaUhMyg xPiA== X-Gm-Message-State: APzg51Ci+kyGJW9QUlJhO0cjx7tHFyyUNw3QsL44ygWQvAiGipA44uff VjNn7mU7jRo6vnCsk6UqIADjuKPTzg/Qpi/Xgb/ZVQ== X-Google-Smtp-Source: ANB0VdYekGrV+/alF5DyuwAFFE1EfEkIkngZ7+eF3jtbLkxOY8kFDeO3/v57nIx+VDNyRPyWFvl6pJHJaDI4uuOve90= X-Received: by 2002:aed:25b4:: with SMTP id x49-v6mr5065556qtc.228.1537725764809; Sun, 23 Sep 2018 11:02:44 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20180923152546.GA6201@lilyforest.localdomain> 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> From: Mikael Magnusson Date: Sun, 23 Sep 2018 20:02:44 +0200 Message-ID: Subject: Re: No fsync on history file? I lost my history To: lilydjwg Cc: Daniel Shahaf , Zsh Users Content-Type: text/plain; charset="UTF-8" On Sun, Sep 23, 2018 at 5:25 PM, lilydjwg 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 > 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