Re: [PATCH 0/3] Supplements to "packed_ref_cache: don't use mmap() for small files"

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 17, 2018, 21:52 UTC
Message-ID: <xmqqvag0qhjg.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <cover.1516017331.git.mhagger@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> writes:

Show 17 quoted lines

> So I will follow up this email with three patches:
>
> 1. Mention that `snapshot::buf` can be NULL for empty files
>
>    I suggest squashing this into your patch, to make it clear that
>    `snapshot::buf` and `snapshot::eof` can also be NULL if the
>    `packed-refs` file is empty.
>
> 2. create_snapshot(): exit early if the file was empty
>
>    Avoid undefined behavior by returning early if `snapshot->buf` is
>    NULL.
>
> 3. find_reference_location(): don't invoke if `snapshot->buf` is NULL
>
>    Avoid undefined behavior and confusing semantics by not calling
>    `find_reference_location()` when `snapshot->buf` is NULL.

These look all sensible with today's code and with v2 from this thread.

With the v3, i.e. "do the xmalloc() even for size==0", however, snapshot->buf would never be NULL, so I'd shelve them for now, though.

Thanks.