Thanks for your patch. I haven't measured the performance difference of `mmap()` vs. `read()` for small `packed-refs` files, but it's not surprising that `read()` would be faster. I especially like the fix for zero-length `packed-refs` files. (Even though AFAIK Git never writes such files, they are totally legitimate and shouldn't cause Git to fail.) With or without the additions mentioned below, Reviewed-by: Michael Haggerty <mhagger@xxxxxxxxxxxx> While reviewing your patch, I realized that some areas of the existing code use constructs that are undefined according to the C standard, such as computing `NULL + 0` and `NULL - NULL`. This was already wrong (and would come up more frequently after your change). Even though these are unlikely to be problems in the real world, it would be good to avoid them. 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. Michael Michael Haggerty (3): SQUASH? Mention that `snapshot::buf` can be NULL for empty files create_snapshot(): exit early if the file was empty find_reference_location(): don't invoke if `snapshot->buf` is NULL refs/packed-backend.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) -- 2.14.2
