git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Jonathan Tan <jonathantanmy@google.com>
Subject: [PATCH 09/16] refs-internal.h: reorganize DO_FOR_EACH_* flag documentation
Date: Fri, 24 Sep 2021 14:39:44 -0400	[thread overview]
Message-ID: <YU4bcPAsVa61t8Ze@coredump.intra.peff.net> (raw)
In-Reply-To: <YU4ZOF9+ubmoItmK@coredump.intra.peff.net>

The documentation for the DO_FOR_EACH_* flags is sprinkled over the
refs-internal.h file. We define the two flags in one spot, and then
describe them in more detail far away from there, in the definitions of
refs_ref_iterator_begin() and ref_iterator_advance_fn().

Let's try to organize this a bit better:

  - convert the #defines to an enum. This makes it clear that they are
    related, and that the enum shows the complete set of flags.

  - combine all descriptions for each flag in a single spot, next to the
    flag's definition

  - use the enum rather than a bare int for functions which take the
    flags. This helps readers realize which flags can be used.

  - clarify the mention of flags for ref_iterator_advance_fn(). It does
    not take flags itself, but is meant to depend on ones set up
    earlier.

Signed-off-by: Jeff King <peff@peff.net>
---
 refs.c               | 10 ++++++----
 refs/refs-internal.h | 46 ++++++++++++++++++++++++++------------------
 2 files changed, 33 insertions(+), 23 deletions(-)

diff --git a/refs.c b/refs.c
index 8b9f7c3a80..c28bd6a818 100644
--- a/refs.c
+++ b/refs.c
@@ -1413,7 +1413,8 @@ int head_ref(each_ref_fn fn, void *cb_data)
 
 struct ref_iterator *refs_ref_iterator_begin(
 		struct ref_store *refs,
-		const char *prefix, int trim, int flags)
+		const char *prefix, int trim,
+		enum do_for_each_ref_flags flags)
 {
 	struct ref_iterator *iter;
 
@@ -1479,7 +1480,8 @@ static int do_for_each_ref_helper(struct repository *r,
 }
 
 static int do_for_each_ref(struct ref_store *refs, const char *prefix,
-			   each_ref_fn fn, int trim, int flags, void *cb_data)
+			   each_ref_fn fn, int trim,
+			   enum do_for_each_ref_flags flags, void *cb_data)
 {
 	struct ref_iterator *iter;
 	struct do_for_each_ref_help hp = { fn, cb_data };
@@ -1516,7 +1518,7 @@ int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)
 
 int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, unsigned int broken)
 {
-	unsigned int flag = 0;
+	enum do_for_each_ref_flags flag = 0;
 
 	if (broken)
 		flag = DO_FOR_EACH_INCLUDE_BROKEN;
@@ -1528,7 +1530,7 @@ int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix,
 			     each_ref_fn fn, void *cb_data,
 			     unsigned int broken)
 {
-	unsigned int flag = 0;
+	enum do_for_each_ref_flags flag = 0;
 
 	if (broken)
 		flag = DO_FOR_EACH_INCLUDE_BROKEN;
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 7b30910974..2c4e1739f2 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -245,16 +245,30 @@ int refs_rename_ref_available(struct ref_store *refs,
 /* We allow "recursive" symbolic refs. Only within reason, though */
 #define SYMREF_MAXDEPTH 5
 
-/* Include broken references in a do_for_each_ref*() iteration: */
-#define DO_FOR_EACH_INCLUDE_BROKEN 0x01
-
 /*
- * Only include per-worktree refs in a do_for_each_ref*() iteration.
- * Normally this will be used with a files ref_store, since that's
- * where all reference backends will presumably store their
- * per-worktree refs.
+ * These flags are passed to refs_ref_iterator_begin() (and do_for_each_ref(),
+ * which feeds it).
  */
-#define DO_FOR_EACH_PER_WORKTREE_ONLY 0x02
+enum do_for_each_ref_flags {
+	/*
+	 * Include broken references in a do_for_each_ref*() iteration, which
+	 * would normally be omitted. This includes both refs that point to
+	 * missing objects (a true repository corruption), ones with illegal
+	 * names (which we prefer not to expose to callers), as well as
+	 * dangling symbolic refs (i.e., those that point to a non-existent
+	 * ref; this is not a corruption, but as they have no valid oid, we
+	 * omit them from normal iteration results).
+	 */
+	DO_FOR_EACH_INCLUDE_BROKEN = (1 << 0),
+
+	/*
+	 * Only include per-worktree refs in a do_for_each_ref*() iteration.
+	 * Normally this will be used with a files ref_store, since that's
+	 * where all reference backends will presumably store their
+	 * per-worktree refs.
+	 */
+	DO_FOR_EACH_PER_WORKTREE_ONLY = (1 << 1),
+};
 
 /*
  * Reference iterators
@@ -357,16 +371,12 @@ int is_empty_ref_iterator(struct ref_iterator *ref_iterator);
  * Return an iterator that goes over each reference in `refs` for
  * which the refname begins with prefix. If trim is non-zero, then
  * trim that many characters off the beginning of each refname.
- * The output is ordered by refname. The following flags are supported:
- *
- * DO_FOR_EACH_INCLUDE_BROKEN: include broken references in
- *         the iteration.
- *
- * DO_FOR_EACH_PER_WORKTREE_ONLY: only produce REF_TYPE_PER_WORKTREE refs.
+ * The output is ordered by refname.
  */
 struct ref_iterator *refs_ref_iterator_begin(
 		struct ref_store *refs,
-		const char *prefix, int trim, int flags);
+		const char *prefix, int trim,
+		enum do_for_each_ref_flags flags);
 
 /*
  * A callback function used to instruct merge_ref_iterator how to
@@ -454,10 +464,8 @@ void base_ref_iterator_free(struct ref_iterator *iter);
 /*
  * backend-specific implementation of ref_iterator_advance. For symrefs, the
  * function should set REF_ISSYMREF, and it should also dereference the symref
- * to provide the OID referent. If DO_FOR_EACH_INCLUDE_BROKEN is set, symrefs
- * with non-existent referents and refs pointing to non-existent object names
- * should also be returned. If DO_FOR_EACH_PER_WORKTREE_ONLY, only
- * REF_TYPE_PER_WORKTREE refs should be returned.
+ * to provide the OID referent. It should respect do_for_each_ref_flags
+ * that were passed to refs_ref_iterator_begin().
  */
 typedef int ref_iterator_advance_fn(struct ref_iterator *ref_iterator);
 
-- 
2.33.0.1071.gb37e412355


  parent reply	other threads:[~2021-09-24 18:39 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-24 18:30 [PATCH 0/16] enabling GIT_REF_PARANOIA by default Jeff King
2021-09-24 18:32 ` [PATCH 01/16] t7900: clean up some more broken refs Jeff King
2021-09-27 17:38   ` Jonathan Tan
2021-09-27 19:49     ` Jeff King
2021-09-24 18:33 ` [PATCH 02/16] t5516: don't use HEAD ref for invalid ref-deletion tests Jeff King
2021-09-24 18:34 ` [PATCH 03/16] t5600: provide detached HEAD for corruption failures Jeff King
2021-09-24 18:35 ` [PATCH 04/16] t5312: drop "verbose" helper Jeff King
2021-09-24 18:36 ` [PATCH 05/16] t5312: create bogus ref as necessary Jeff King
2021-09-24 18:36 ` [PATCH 06/16] t5312: test non-destructive repack Jeff King
2021-09-24 18:37 ` [PATCH 07/16] t5312: be more assertive about command failure Jeff King
2021-09-24 18:37 ` [PATCH 08/16] refs-internal.h: move DO_FOR_EACH_* flags next to each other Jeff King
2021-09-24 18:39 ` Jeff King [this message]
2021-09-24 18:41 ` [PATCH 10/16] refs: add DO_FOR_EACH_OMIT_DANGLING_SYMREFS flag Jeff King
2021-09-24 18:42 ` [PATCH 11/16] refs: omit dangling symrefs when using GIT_REF_PARANOIA Jeff King
2021-09-24 18:46 ` [PATCH 12/16] refs: turn on GIT_REF_PARANOIA by default Jeff King
2021-09-27 17:42   ` Jonathan Tan
2021-09-24 18:46 ` [PATCH 13/16] repack, prune: drop GIT_REF_PARANOIA settings Jeff King
2021-09-24 18:48 ` [PATCH 14/16] ref-filter: stop setting FILTER_REFS_INCLUDE_BROKEN Jeff King
2021-09-24 18:48 ` [PATCH 15/16] ref-filter: drop broken-ref code entirely Jeff King
2021-09-24 18:48 ` [PATCH 16/16] refs: drop "broken" flag from for_each_fullref_in() Jeff King
2021-09-27 17:47   ` Jonathan Tan
2021-09-24 20:22 ` [PATCH 0/16] enabling GIT_REF_PARANOIA by default 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=YU4bcPAsVa61t8Ze@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=jonathantanmy@google.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.
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).