git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Michael Haggerty <mhagger@alum.mit.edu>
To: Kim Gybels <kgybels@infogroep.be>
Cc: Johannes Schindelin <johannes.schindelin@gmx.de>,
	Junio C Hamano <gitster@pobox.com>,
	git@vger.kernel.org, Michael Haggerty <mhagger@alum.mit.edu>
Subject: [PATCH 3/3] find_reference_location(): don't invoke if `snapshot->buf` is NULL
Date: Mon, 15 Jan 2018 13:17:35 +0100	[thread overview]
Message-ID: <46a457904cf0261e337dfd94dc2f1d62abf64053.1516017331.git.mhagger@alum.mit.edu> (raw)
In-Reply-To: <20180114191416.2368-1-kgybels@infogroep.be>
In-Reply-To: <cover.1516017331.git.mhagger@alum.mit.edu>

If `snapshot->buf` is NULL, then `find_reference_location()` has two
problems:

1. It relies on behavior that is technically undefined in C, such as
   computing `NULL + 0`.

2. It returns NULL if the reference doesn't exist, even if `mustexist`
   is not set. This problem doesn't come up in the current code,
   because we never call this function with `snapshot->buf == NULL`
   and `mustexist` set. But it is something that future callers need
   to be aware of.

We could fix the first problem by adding some extra logic to the
function. But considering both problems together, it is more
straightforward to document that the function should only be called if
`snapshot->buf` is non-NULL.

Adjust `packed_read_raw_ref()` to return early if `snapshot->buf` is
NULL rather than calling `find_reference_location()`.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs/packed-backend.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 36796d65f0..ed2b396bef 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -521,8 +521,9 @@ static int load_contents(struct snapshot *snapshot)
  * reference name; for example, one could search for "refs/replace/"
  * to find the start of any replace references.
  *
+ * This function must only be called if `snapshot->buf` is non-NULL.
  * The record is sought using a binary search, so `snapshot->buf` must
- * be sorted.
+ * also be sorted.
  */
 static const char *find_reference_location(struct snapshot *snapshot,
 					   const char *refname, int mustexist)
@@ -728,6 +729,12 @@ static int packed_read_raw_ref(struct ref_store *ref_store,
 
 	*type = 0;
 
+	if (!snapshot->buf) {
+		/* There are no packed references */
+		errno = ENOENT;
+		return -1;
+	}
+
 	rec = find_reference_location(snapshot, refname, 1);
 
 	if (!rec) {
-- 
2.14.2


  parent reply	other threads:[~2018-01-15 12:18 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-13 16:11 [PATCH] packed_ref_cache: don't use mmap() for small files Kim Gybels
2018-01-13 18:56 ` Johannes Schindelin
2018-01-14 19:14 ` [PATCH v2] " Kim Gybels
2018-01-15 12:17   ` [PATCH 0/3] Supplements to "packed_ref_cache: don't use mmap() for small files" Michael Haggerty
2018-01-15 12:17     ` [PATCH 1/3] SQUASH? Mention that `snapshot::buf` can be NULL for empty files Michael Haggerty
2018-01-15 12:17     ` [PATCH 2/3] create_snapshot(): exit early if the file was empty Michael Haggerty
2018-01-15 12:17     ` Michael Haggerty [this message]
2018-01-17 20:23     ` [PATCH 0/3] Supplements to "packed_ref_cache: don't use mmap() for small files" Johannes Schindelin
2018-01-17 21:52     ` Junio C Hamano
2018-01-15 21:15 ` [PATCH] packed_ref_cache: don't use mmap() for small files Jeff King
2018-01-15 23:37   ` Kim Gybels
2018-01-15 23:52     ` Jeff King
2018-01-16 19:38       ` [PATCH v3] " Kim Gybels
2018-01-17 22:09         ` Jeff King
2018-01-21  4:41           ` Michael Haggerty
2018-01-22 19:31             ` Junio C Hamano
2018-01-24 11:05               ` Michael Haggerty
2018-01-24 11:14                 ` [PATCH 0/6] Yet another approach to handling empty snapshots Michael Haggerty
2018-01-24 11:14                   ` [PATCH 1/6] struct snapshot: store `start` rather than `header_len` Michael Haggerty
2018-01-24 20:36                     ` Jeff King
2018-01-24 11:14                   ` [PATCH 2/6] create_snapshot(): use `xmemdupz()` rather than a strbuf Michael Haggerty
2018-01-24 11:14                   ` [PATCH 3/6] find_reference_location(): make function safe for empty snapshots Michael Haggerty
2018-01-24 20:27                     ` Jeff King
2018-01-24 21:11                       ` Junio C Hamano
2018-01-24 21:34                         ` Jeff King
2018-01-24 11:14                   ` [PATCH 4/6] packed_ref_iterator_begin(): make optimization more general Michael Haggerty
2018-01-24 20:32                     ` Jeff King
2018-01-24 11:14                   ` [PATCH 5/6] load_contents(): don't try to mmap an empty file Michael Haggerty
2018-01-24 11:14                   ` [PATCH 6/6] packed_ref_cache: don't use mmap() for small files Michael Haggerty
2018-01-24 20:38                   ` [PATCH 0/6] Yet another approach to handling empty snapshots Jeff King
2018-01-24 20:54                   ` Junio C Hamano
2018-02-15 16:54                   ` Johannes Schindelin
2018-01-24 18:05                 ` [PATCH v3] packed_ref_cache: don't use mmap() for small files Junio C Hamano

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=46a457904cf0261e337dfd94dc2f1d62abf64053.1516017331.git.mhagger@alum.mit.edu \
    --to=mhagger@alum.mit.edu \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=kgybels@infogroep.be \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).