diff mbox series

[v2,1/6] trailer: separate public from internal portion of trailer_iterator

Message ID 4f116d2550f6cf218477560a9e25dbe4c384a2a6.1694240177.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Commit 13211ae23f9126be81b3b483163bf963df4826aa
Headers show
Series Trailer readability cleanups | expand

Commit Message

Linus Arver Sept. 9, 2023, 6:16 a.m. UTC
From: Linus Arver <linusa@google.com>

The fields here are not meant to be used by downstream callers, so put
them behind an anonymous struct named as "internal" to warn against
their use. This follows the pattern in 576de3d956 (unpack_trees: start
splitting internal fields from public API, 2023-02-27).

Signed-off-by: Linus Arver <linusa@google.com>
---
 trailer.c | 10 +++++-----
 trailer.h |  6 ++++--
 2 files changed, 9 insertions(+), 7 deletions(-)

Comments

Junio C Hamano Sept. 11, 2023, 5:10 p.m. UTC | #1
"Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Linus Arver <linusa@google.com>
>
> The fields here are not meant to be used by downstream callers, so put
> them behind an anonymous struct named as "internal" to warn against
> their use. This follows the pattern in 576de3d956 (unpack_trees: start
> splitting internal fields from public API, 2023-02-27).

OK.  The patch shows that there exist no code external to this file
that touch these members that are marked "private", so it has some
auditing value from that point of view, which is nice.

But that is only about today's code and does not protect us from
future breakage.  In other words, "git grep internal\\." would not
be an effective way to find misuses of these members from the
sidelines.  But that is OK, as "git grep -E '([.]|->)info'" would
not be an effective way in today's code, either, and the patch is
not making things worse.

Queued.  Thanks.

> Signed-off-by: Linus Arver <linusa@google.com>
> ---
>  trailer.c | 10 +++++-----
>  trailer.h |  6 ++++--
>  2 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/trailer.c b/trailer.c
> index f408f9b058d..de4bdece847 100644
> --- a/trailer.c
> +++ b/trailer.c
> @@ -1220,14 +1220,14 @@ void trailer_iterator_init(struct trailer_iterator *iter, const char *msg)
>  	strbuf_init(&iter->key, 0);
>  	strbuf_init(&iter->val, 0);
>  	opts.no_divider = 1;
> -	trailer_info_get(&iter->info, msg, &opts);
> -	iter->cur = 0;
> +	trailer_info_get(&iter->internal.info, msg, &opts);
> +	iter->internal.cur = 0;
>  }
>  
>  int trailer_iterator_advance(struct trailer_iterator *iter)
>  {
> -	while (iter->cur < iter->info.trailer_nr) {
> -		char *trailer = iter->info.trailers[iter->cur++];
> +	while (iter->internal.cur < iter->internal.info.trailer_nr) {
> +		char *trailer = iter->internal.info.trailers[iter->internal.cur++];
>  		int separator_pos = find_separator(trailer, separators);
>  
>  		if (separator_pos < 1)
> @@ -1245,7 +1245,7 @@ int trailer_iterator_advance(struct trailer_iterator *iter)
>  
>  void trailer_iterator_release(struct trailer_iterator *iter)
>  {
> -	trailer_info_release(&iter->info);
> +	trailer_info_release(&iter->internal.info);
>  	strbuf_release(&iter->val);
>  	strbuf_release(&iter->key);
>  }
> diff --git a/trailer.h b/trailer.h
> index 795d2fccfd9..ab2cd017567 100644
> --- a/trailer.h
> +++ b/trailer.h
> @@ -119,8 +119,10 @@ struct trailer_iterator {
>  	struct strbuf val;
>  
>  	/* private */
> -	struct trailer_info info;
> -	size_t cur;
> +	struct {
> +		struct trailer_info info;
> +		size_t cur;
> +	} internal;
>  };
>  
>  /*
diff mbox series

Patch

diff --git a/trailer.c b/trailer.c
index f408f9b058d..de4bdece847 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1220,14 +1220,14 @@  void trailer_iterator_init(struct trailer_iterator *iter, const char *msg)
 	strbuf_init(&iter->key, 0);
 	strbuf_init(&iter->val, 0);
 	opts.no_divider = 1;
-	trailer_info_get(&iter->info, msg, &opts);
-	iter->cur = 0;
+	trailer_info_get(&iter->internal.info, msg, &opts);
+	iter->internal.cur = 0;
 }
 
 int trailer_iterator_advance(struct trailer_iterator *iter)
 {
-	while (iter->cur < iter->info.trailer_nr) {
-		char *trailer = iter->info.trailers[iter->cur++];
+	while (iter->internal.cur < iter->internal.info.trailer_nr) {
+		char *trailer = iter->internal.info.trailers[iter->internal.cur++];
 		int separator_pos = find_separator(trailer, separators);
 
 		if (separator_pos < 1)
@@ -1245,7 +1245,7 @@  int trailer_iterator_advance(struct trailer_iterator *iter)
 
 void trailer_iterator_release(struct trailer_iterator *iter)
 {
-	trailer_info_release(&iter->info);
+	trailer_info_release(&iter->internal.info);
 	strbuf_release(&iter->val);
 	strbuf_release(&iter->key);
 }
diff --git a/trailer.h b/trailer.h
index 795d2fccfd9..ab2cd017567 100644
--- a/trailer.h
+++ b/trailer.h
@@ -119,8 +119,10 @@  struct trailer_iterator {
 	struct strbuf val;
 
 	/* private */
-	struct trailer_info info;
-	size_t cur;
+	struct {
+		struct trailer_info info;
+		size_t cur;
+	} internal;
 };
 
 /*