From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=MAILING_LIST_MULTI autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 26618 invoked from network); 7 Oct 2021 09:36:12 -0000 Received: from lists.zx2c4.com (165.227.139.114) by inbox.vuxu.org with ESMTPUTF8; 7 Oct 2021 09:36:12 -0000 Received: by lists.zx2c4.com (ZX2C4 Mail Server) with ESMTP id fe25a4c4; Thu, 7 Oct 2021 09:35:58 +0000 (UTC) Return-Path: Received: from mta02.prd.rdg.aluminati.org (mta02.prd.rdg.aluminati.org [94.76.243.215]) by lists.zx2c4.com (ZX2C4 Mail Server) with ESMTPS id 8dc90427 (TLSv1.3:AEAD-AES256-GCM-SHA384:256:NO) for ; Thu, 7 Oct 2021 09:35:57 +0000 (UTC) Received: from mta02.prd.rdg.aluminati.org (localhost [127.0.0.1]) by mta.aluminati.local (Postfix) with ESMTP id 4BD781FF5E; Thu, 7 Oct 2021 10:35:57 +0100 (BST) Received: from localhost (localhost [127.0.0.1]) by mta02.prd.rdg.aluminati.org (Postfix) with ESMTP id 4690A3C3; Thu, 7 Oct 2021 10:35:57 +0100 (BST) X-Quarantine-ID: X-Virus-Scanned: Debian amavisd-new at mta02.prd.rdg.aluminati.org Received: from mta.aluminati.local ([127.0.0.1]) by localhost (mta02.prd.rdg.aluminati.org [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id hydgnJ95d7sd; Thu, 7 Oct 2021 10:35:55 +0100 (BST) Received: from keeping.me.uk (unknown [81.174.171.191]) by svc01-2.prd.rdg.aluminati.org (Postfix) with ESMTPSA id 22C12780002; Thu, 7 Oct 2021 10:35:53 +0100 (BST) Date: Thu, 7 Oct 2021 10:35:49 +0100 From: John Keeping To: Hristo Venev Cc: cgit@lists.zx2c4.com Subject: Re: [PATCH] cache: Tolerate short writes in print_slot Message-ID: References: <20210910141841.2092532-1-hristo@venev.name> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210910141841.2092532-1-hristo@venev.name> X-BeenThere: cgit@lists.zx2c4.com X-Mailman-Version: 2.1.30rc1 Precedence: list List-Id: List for cgit developers and users List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: cgit-bounces@lists.zx2c4.com Sender: "CGit" On Fri, Sep 10, 2021 at 05:18:41PM +0300, Hristo Venev wrote: > sendfile() can return after a short read/write, so we may need to call > it more than once. Furthermore, not all files support sendfile(), so we > may need to fall back to read/write. Have you seen these errors in practice, or is this just theoretical? In recent (since v2.6.33) versions of Linux, all files should support sendfile(), especially since we expect out_fd to be a socket or pipe. > On the read/write path, use write_in_full which deals with short writes. > > Signed-off-by: Hristo Venev > --- > cache.c | 46 ++++++++++++++++++++++++---------------------- > 1 file changed, 24 insertions(+), 22 deletions(-) > > diff --git a/cache.c b/cache.c > index 55199e8..85cfbd9 100644 > --- a/cache.c > +++ b/cache.c > @@ -85,40 +85,42 @@ static int close_slot(struct cache_slot *slot) > /* Print the content of the active cache slot (but skip the key). */ > static int print_slot(struct cache_slot *slot) > { > -#ifdef HAVE_LINUX_SENDFILE > - off_t start_off; > - int ret; > + off_t off; > + ssize_t i; > + > + off = slot->keylen + 1; > > - start_off = slot->keylen + 1; > +#ifdef HAVE_LINUX_SENDFILE > + off_t size; decl-after-stmt if HAVE_LINUX_SENDFILE is set. > + size = slot->cache_st.st_size; > > do { > - ret = sendfile(STDOUT_FILENO, slot->cache_fd, &start_off, > - slot->cache_st.st_size - start_off); > - if (ret < 0) { > + i = sendfile(STDOUT_FILENO, slot->cache_fd, &off, size - off); Why is ret renamed? i is normally a loop index variable, using it for the return value here is strange, please stick with "ret". > + if (i < 0) { > if (errno == EAGAIN || errno == EINTR) > continue; > + /* Fall back to read/write on EINVAL */ > + if (errno == EINVAL) > + break; > return errno; > } > - return 0; > + if (off == size) > + return 0; > } while (1); > -#else > - ssize_t i, j; > +#endif > > - i = lseek(slot->cache_fd, slot->keylen + 1, SEEK_SET); > - if (i != slot->keylen + 1) > + if (lseek(slot->cache_fd, off, SEEK_SET) != off) > return errno; > > do { > - i = j = xread(slot->cache_fd, slot->buf, sizeof(slot->buf)); > - if (i > 0) > - j = xwrite(STDOUT_FILENO, slot->buf, i); > - } while (i > 0 && j == i); > - > - if (i < 0 || j != i) > - return errno; > - else > - return 0; > -#endif > + i = xread(slot->cache_fd, slot->buf, sizeof(slot->buf)); > + if (i < 0) > + return errno; > + if (i == 0) > + return 0; > + if (write_in_full(STDOUT_FILENO, slot->buf, i) < 0) > + return errno; > + } while (1); > } > > /* Check if the slot has expired */ > -- > 2.31.1 >