Re: [PATCH] update-server-info: avoid needless overwrites

From: Eric Sunshine <sunshine@sunshineco.com>
Date: May 11, 2019, 07:35 UTC
Message-ID: <CAPig+cRZy4yM+ZuHhZK3tPV_QRvSy63aHBZZj6AcFTu+5FNG0A@mail.gmail.com>
In-Reply-To: <20190511013455.5886-1-e@80x24.org>

On Fri, May 10, 2019 at 9:35 PM Eric Wong <e@80x24.org> wrote:

Show 16 quoted lines

> Do not change the existing info/refs and objects/info/packs
> files if they match the existing content on the filesystem.
> This is intended to preserve mtime and make it easier for dumb
> HTTP pollers to rely on the If-Modified-Since header.
> [...]
> Signed-off-by: Eric Wong <e@80x24.org>
> ---
> diff --git a/server-info.c b/server-info.c
> @@ -6,37 +6,78 @@
> +static int files_differ(FILE *fp, const char *path)
> +{
> +       [...]
> +       struct strbuf tmp = STRBUF_INIT;
> +       [...]
> +       if (strbuf_fread(&tmp, (size_t)new_len, fp) != (size_t)new_len)
> +               return 1;

Although strbuf_fread() will release 'tmp' automatically if the read actually fails, it won't do so otherwise. So, if it reads more or fewer bytes than expected, for some reason, then this early return will leak the strbuf, won't it?

Show 7 quoted lines

> +       the_hash_algo->init_fn(&c);
> +       the_hash_algo->update_fn(&c, tmp.buf, tmp.len);
> +       the_hash_algo->final_fn(oid_new.hash, &c);
> +       strbuf_release(&tmp);
> +
> +       if (strbuf_read_file(&tmp, path, (size_t)st.st_size) < 0)
> +               return 1;

Same issue.

> diff --git a/t/t5200-update-server-info.sh b/t/t5200-update-server-info.sh
> @@ -0,0 +1,41 @@
> +test_description='Test git stash show configuration.'

Copy/paste error for test description?