List for cgit developers and users
 help / color / mirror / Atom feed
From: john at keeping.me.uk (John Keeping)
Subject: [PATCH] cache: flush stdio before restoring FDs
Date: Mon, 24 Apr 2017 19:52:40 +0100	[thread overview]
Message-ID: <20170424185240.GB1788@john.keeping.me.uk> (raw)
In-Reply-To: <20170424155440.GB15623@gmail.com>

As described in commit 2efb59e (ui-patch: Flush stdout after outputting
data, 2014-06-11), we need to ensure that stdout is flushed before
restoring the file descriptor when writing to the cache.  It turns out
that it's not just ui-patch that is affected by this but also raw diff
which writes to stdout internally.

Let's avoid risking more places doing this by ensuring that stdout is
flushed after writing in fill_slot().

Signed-off-by: John Keeping <john at keeping.me.uk>
---
On Mon, Apr 24, 2017 at 11:54:40AM -0400, Konstantin Ryabitsev wrote:
> Very reminiscent of a fix for ui-patch.c that fixed patch truncation [1]
> there's a similar problem with cached rawdiff output. E.g.:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/rawdiff/?id=v4.11-rc7&id2=v4.11-rc8
> 
> You will notice that the output is truncated (if it isn't just reload to 
> get the cached version).
> 
> I would love a speedy fix, as we're trying to use this in place of 
> publishing pre-generated patches.

I guess it's the same problem we fixed before for ui-patch, so here's a
patch to move the fix into the common path for all pages.

 cache.c    | 6 ++++++
 ui-patch.c | 2 --
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/cache.c b/cache.c
index 6736a01..2ccdc4e 100644
--- a/cache.c
+++ b/cache.c
@@ -224,6 +224,12 @@ static int fill_slot(struct cache_slot *slot)
 	/* Generate cache content */
 	slot->fn();
 
+	/* Make sure any buffered data is flushed to the file */
+	if (fflush(stdout)) {
+		close(tmp);
+		return errno;
+	}
+
 	/* update stat info */
 	if (fstat(slot->lock_fd, &slot->cache_st)) {
 		close(tmp);
diff --git a/ui-patch.c b/ui-patch.c
index 047e2f9..6745b69 100644
--- a/ui-patch.c
+++ b/ui-patch.c
@@ -92,6 +92,4 @@ void cgit_print_patch(const char *new_rev, const char *old_rev,
 		log_tree_commit(&rev, commit);
 		printf("-- \ncgit %s\n\n", cgit_version);
 	}
-
-	fflush(stdout);
 }
-- 
2.12.2.648.g6730d8bc62.dirty



  reply	other threads:[~2017-04-24 18:52 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-24 15:54 Bug: rawdiff output truncated on cache save mricon
2017-04-24 18:52 ` john [this message]
2017-04-24 18:55   ` [PATCH] cache: flush stdio before restoring FDs Jason
2017-04-28 16:36   ` mricon
2017-09-25 12:51   ` mricon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170424185240.GB1788@john.keeping.me.uk \
    --to=cgit@lists.zx2c4.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).