git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH 0/10] dropping more unused function parameters
@ 2019-02-14  5:47 Jeff King
  2019-02-14  5:48 ` [PATCH 01/10] diff: drop options parameter from diffcore_fix_diff_index() Jeff King
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Jeff King @ 2019-02-14  5:47 UTC (permalink / raw)
  To: git

Here are ten more patches from my exploration of -Wunused-parameters.
Most of these are quite old and not urgent, but things seem relatively
quiet now, and they all merge cleanly with "pu".

In a sense they should all be trivial to review, as they only remove
unused parameters. But the thing to consider is whether that parameter's
lack of use is actually a bug, or if it's simply cruft (and I argue in
each case in favor of cruft).

  [01/10]: diff: drop options parameter from diffcore_fix_diff_index()
  [02/10]: diff: drop unused color reset parameters
  [03/10]: diff: drop unused emit data parameter from sane_truncate_line()
  [04/10]: diff: drop complete_rewrite parameter from run_external_diff()
  [05/10]: merge-recursive: drop several unused parameters
  [06/10]: pack-objects: drop unused parameter from oe_map_new_pack()
  [07/10]: files-backend: drop refs parameter from split_symref_update()
  [08/10]: ref-filter: drop unused buf/sz pairs
  [09/10]: ref-filter: drop unused "obj" parameters
  [10/10]: ref-filter: drop unused "sz" parameters

 diff-lib.c           |  2 +-
 diff.c               | 31 ++++++++++++-------------------
 diff.h               |  2 +-
 merge-recursive.c    | 19 +++++++------------
 pack-objects.c       |  3 +--
 pack-objects.h       |  6 +++---
 ref-filter.c         | 36 ++++++++++++++++++------------------
 refs/files-backend.c |  5 ++---
 8 files changed, 45 insertions(+), 59 deletions(-)

-Peff

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 01/10] diff: drop options parameter from diffcore_fix_diff_index()
  2019-02-14  5:47 [PATCH 0/10] dropping more unused function parameters Jeff King
@ 2019-02-14  5:48 ` Jeff King
  2019-02-14  5:48 ` [PATCH 02/10] diff: drop unused color reset parameters Jeff King
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Jeff King @ 2019-02-14  5:48 UTC (permalink / raw)
  To: git

The sole purpose of this function is to fix the sorting order of the
queued diff entries. It doesn't need to know about any diff options, so
we can drop the unused parameter.

Signed-off-by: Jeff King <peff@peff.net>
---
 diff-lib.c | 2 +-
 diff.c     | 2 +-
 diff.h     | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/diff-lib.c b/diff-lib.c
index 23c8d351b3..a838c219ec 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -531,7 +531,7 @@ int run_diff_index(struct rev_info *revs, int cached)
 		exit(128);
 
 	diff_set_mnemonic_prefix(&revs->diffopt, "c/", cached ? "i/" : "w/");
-	diffcore_fix_diff_index(&revs->diffopt);
+	diffcore_fix_diff_index();
 	diffcore_std(&revs->diffopt);
 	diff_flush(&revs->diffopt);
 	trace_performance_leave("diff-index");
diff --git a/diff.c b/diff.c
index 5306c48652..3e7d11fb70 100644
--- a/diff.c
+++ b/diff.c
@@ -6224,7 +6224,7 @@ static int diffnamecmp(const void *a_, const void *b_)
 	return strcmp(name_a, name_b);
 }
 
-void diffcore_fix_diff_index(struct diff_options *options)
+void diffcore_fix_diff_index(void)
 {
 	struct diff_queue_struct *q = &diff_queued_diff;
 	QSORT(q->queue, q->nr, diffnamecmp);
diff --git a/diff.h b/diff.h
index b512d0477a..8f55aad5ba 100644
--- a/diff.h
+++ b/diff.h
@@ -367,7 +367,7 @@ int git_config_rename(const char *var, const char *value);
 #define DIFF_PICKAXE_IGNORE_CASE	32
 
 void diffcore_std(struct diff_options *);
-void diffcore_fix_diff_index(struct diff_options *);
+void diffcore_fix_diff_index(void);
 
 #define COMMON_DIFF_OPTIONS_HELP \
 "\ncommon diff options:\n" \
-- 
2.21.0.rc1.567.g648f076c3f


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 02/10] diff: drop unused color reset parameters
  2019-02-14  5:47 [PATCH 0/10] dropping more unused function parameters Jeff King
  2019-02-14  5:48 ` [PATCH 01/10] diff: drop options parameter from diffcore_fix_diff_index() Jeff King
@ 2019-02-14  5:48 ` Jeff King
  2019-02-14  5:48 ` [PATCH 03/10] diff: drop unused emit data parameter from sane_truncate_line() Jeff King
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Jeff King @ 2019-02-14  5:48 UTC (permalink / raw)
  To: git

Several of the emit_* functions take a "reset" color parameter, but
never actually look at it (instead, they call into emit_diff_symbol,
which handles the colors itself). Let's drop these unused parameters.

Note that emit_line() does still take a color/reset pair, and actually
uses it. It cannot be refactored to match these other functions because
it's the thing that emit_diff_symbol eventually calls into (i.e., it
does not by itself know which colors to use, and must be told by the
caller).

Signed-off-by: Jeff King <peff@peff.net>
---
 diff.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/diff.c b/diff.c
index 3e7d11fb70..de45bbd76e 100644
--- a/diff.c
+++ b/diff.c
@@ -1613,8 +1613,7 @@ static int new_blank_line_at_eof(struct emit_callback *ecbdata, const char *line
 	return ws_blank_line(line, len, ecbdata->ws_rule);
 }
 
-static void emit_add_line(const char *reset,
-			  struct emit_callback *ecbdata,
+static void emit_add_line(struct emit_callback *ecbdata,
 			  const char *line, int len)
 {
 	unsigned flags = WSEH_NEW | ecbdata->ws_rule;
@@ -1624,16 +1623,14 @@ static void emit_add_line(const char *reset,
 	emit_diff_symbol(ecbdata->opt, DIFF_SYMBOL_PLUS, line, len, flags);
 }
 
-static void emit_del_line(const char *reset,
-			  struct emit_callback *ecbdata,
+static void emit_del_line(struct emit_callback *ecbdata,
 			  const char *line, int len)
 {
 	unsigned flags = WSEH_OLD | ecbdata->ws_rule;
 	emit_diff_symbol(ecbdata->opt, DIFF_SYMBOL_MINUS, line, len, flags);
 }
 
-static void emit_context_line(const char *reset,
-			      struct emit_callback *ecbdata,
+static void emit_context_line(struct emit_callback *ecbdata,
 			      const char *line, int len)
 {
 	unsigned flags = WSEH_CONTEXT | ecbdata->ws_rule;
@@ -1742,7 +1739,6 @@ static void emit_rewrite_lines(struct emit_callback *ecb,
 			       int prefix, const char *data, int size)
 {
 	const char *endp = NULL;
-	const char *reset = diff_get_color(ecb->color_diff, DIFF_RESET);
 
 	while (0 < size) {
 		int len;
@@ -1751,10 +1747,10 @@ static void emit_rewrite_lines(struct emit_callback *ecb,
 		len = endp ? (endp - data + 1) : size;
 		if (prefix != '+') {
 			ecb->lno_in_preimage++;
-			emit_del_line(reset, ecb, data, len);
+			emit_del_line(ecb, data, len);
 		} else {
 			ecb->lno_in_postimage++;
-			emit_add_line(reset, ecb, data, len);
+			emit_add_line(ecb, data, len);
 		}
 		size -= len;
 		data += len;
@@ -2325,7 +2321,6 @@ static void find_lno(const char *line, struct emit_callback *ecbdata)
 static void fn_out_consume(void *priv, char *line, unsigned long len)
 {
 	struct emit_callback *ecbdata = priv;
-	const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);
 	struct diff_options *o = ecbdata->opt;
 
 	o->found_changes = 1;
@@ -2392,16 +2387,16 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 	switch (line[0]) {
 	case '+':
 		ecbdata->lno_in_postimage++;
-		emit_add_line(reset, ecbdata, line + 1, len - 1);
+		emit_add_line(ecbdata, line + 1, len - 1);
 		break;
 	case '-':
 		ecbdata->lno_in_preimage++;
-		emit_del_line(reset, ecbdata, line + 1, len - 1);
+		emit_del_line(ecbdata, line + 1, len - 1);
 		break;
 	case ' ':
 		ecbdata->lno_in_postimage++;
 		ecbdata->lno_in_preimage++;
-		emit_context_line(reset, ecbdata, line + 1, len - 1);
+		emit_context_line(ecbdata, line + 1, len - 1);
 		break;
 	default:
 		/* incomplete line at the end */
-- 
2.21.0.rc1.567.g648f076c3f


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 03/10] diff: drop unused emit data parameter from sane_truncate_line()
  2019-02-14  5:47 [PATCH 0/10] dropping more unused function parameters Jeff King
  2019-02-14  5:48 ` [PATCH 01/10] diff: drop options parameter from diffcore_fix_diff_index() Jeff King
  2019-02-14  5:48 ` [PATCH 02/10] diff: drop unused color reset parameters Jeff King
@ 2019-02-14  5:48 ` Jeff King
  2019-02-14  5:49 ` [PATCH 04/10] diff: drop complete_rewrite parameter from run_external_diff() Jeff King
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Jeff King @ 2019-02-14  5:48 UTC (permalink / raw)
  To: git

We pass the "struct emit_callback" (which contains all of the context
for our diff) into sane_truncate_line(), but that function doesn't
actually use it. In theory we might eventually develop a diff option
that impacts this, but in the meantime let's not mislead anybody reading
the code. Since the function is static, it would be easy to pass it
again if it should ever become useful.

Signed-off-by: Jeff King <peff@peff.net>
---
 diff.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/diff.c b/diff.c
index de45bbd76e..d24843b531 100644
--- a/diff.c
+++ b/diff.c
@@ -2287,7 +2287,7 @@ const char *diff_line_prefix(struct diff_options *opt)
 	return msgbuf->buf;
 }
 
-static unsigned long sane_truncate_line(struct emit_callback *ecb, char *line, unsigned long len)
+static unsigned long sane_truncate_line(char *line, unsigned long len)
 {
 	const char *cp;
 	unsigned long allot;
@@ -2351,7 +2351,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 	if (line[0] == '@') {
 		if (ecbdata->diff_words)
 			diff_words_flush(ecbdata);
-		len = sane_truncate_line(ecbdata, line, len);
+		len = sane_truncate_line(line, len);
 		find_lno(line, ecbdata);
 		emit_hunk_header(ecbdata, line, len);
 		return;
-- 
2.21.0.rc1.567.g648f076c3f


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 04/10] diff: drop complete_rewrite parameter from run_external_diff()
  2019-02-14  5:47 [PATCH 0/10] dropping more unused function parameters Jeff King
                   ` (2 preceding siblings ...)
  2019-02-14  5:48 ` [PATCH 03/10] diff: drop unused emit data parameter from sane_truncate_line() Jeff King
@ 2019-02-14  5:49 ` Jeff King
  2019-02-14  5:50 ` [PATCH 05/10] merge-recursive: drop several unused parameters Jeff King
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Jeff King @ 2019-02-14  5:49 UTC (permalink / raw)
  To: git

Our builtin_diff() wants to know whether break-detection found a
complete rewrite, because it changes how the diff is shown. However,
when calling out to an external diff, we don't pass this information
along (and doing so would require designing a new interface to the
user-provided program).

Let's drop the unused parameter to make this fact more clear.

Signed-off-by: Jeff King <peff@peff.net>
---
An alternative is to pass $COMPLETE_REWRITE in the environment. That
would avoid disrupting existing external diff callers, and they could
peek at the information if they really cared. But given that nobody has
actually asked for this, I'm inclined not to add a new interface element
that we'd then have to support forever.

 diff.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/diff.c b/diff.c
index d24843b531..e3f263769b 100644
--- a/diff.c
+++ b/diff.c
@@ -4178,7 +4178,6 @@ static void run_external_diff(const char *pgm,
 			      struct diff_filespec *one,
 			      struct diff_filespec *two,
 			      const char *xfrm_msg,
-			      int complete_rewrite,
 			      struct diff_options *o)
 {
 	struct argv_array argv = ARGV_ARRAY_INIT;
@@ -4336,8 +4335,7 @@ static void run_diff_cmd(const char *pgm,
 	}
 
 	if (pgm) {
-		run_external_diff(pgm, name, other, one, two, xfrm_msg,
-				  complete_rewrite, o);
+		run_external_diff(pgm, name, other, one, two, xfrm_msg, o);
 		return;
 	}
 	if (one && two)
-- 
2.21.0.rc1.567.g648f076c3f


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 05/10] merge-recursive: drop several unused parameters
  2019-02-14  5:47 [PATCH 0/10] dropping more unused function parameters Jeff King
                   ` (3 preceding siblings ...)
  2019-02-14  5:49 ` [PATCH 04/10] diff: drop complete_rewrite parameter from run_external_diff() Jeff King
@ 2019-02-14  5:50 ` Jeff King
  2019-02-14  5:50 ` [PATCH 06/10] pack-objects: drop unused parameter from oe_map_new_pack() Jeff King
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Jeff King @ 2019-02-14  5:50 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren

There are a few functions related to directory renames that have unused
parameters. After consulting with the author in [1], these seem to be
leftover cruft from the development process, and not signs of any bug.
Let's drop them.

[1] https://public-inbox.org/git/CABPp-BHobf8wbBsXF97scNQCzkxQukziODRXq6JOOWq61cAd9g@mail.gmail.com/

Helped-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
---
 merge-recursive.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index 4851825aeb..f270fa66f3 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1402,8 +1402,7 @@ static int merge_mode_and_contents(struct merge_options *o,
 
 static int handle_rename_via_dir(struct merge_options *o,
 				 struct diff_filepair *pair,
-				 const char *rename_branch,
-				 const char *other_branch)
+				 const char *rename_branch)
 {
 	/*
 	 * Handle file adds that need to be renamed due to directory rename
@@ -2213,8 +2212,7 @@ static void handle_directory_level_conflicts(struct merge_options *o,
 	remove_hashmap_entries(dir_re_merge, &remove_from_merge);
 }
 
-static struct hashmap *get_directory_renames(struct diff_queue_struct *pairs,
-					     struct tree *tree)
+static struct hashmap *get_directory_renames(struct diff_queue_struct *pairs)
 {
 	struct hashmap *dir_renames;
 	struct hashmap_iter iter;
@@ -2460,8 +2458,7 @@ static void apply_directory_rename_modifications(struct merge_options *o,
 						 struct tree *o_tree,
 						 struct tree *a_tree,
 						 struct tree *b_tree,
-						 struct string_list *entries,
-						 int *clean)
+						 struct string_list *entries)
 {
 	struct string_list_item *item;
 	int stage = (tree == a_tree ? 2 : 3);
@@ -2632,8 +2629,7 @@ static struct string_list *get_renames(struct merge_options *o,
 			apply_directory_rename_modifications(o, pair, new_path,
 							     re, tree, o_tree,
 							     a_tree, b_tree,
-							     entries,
-							     clean_merge);
+							     entries);
 	}
 
 	hashmap_iter_init(&collisions, &iter);
@@ -2944,8 +2940,8 @@ static int detect_and_process_renames(struct merge_options *o,
 	merge_pairs = get_diffpairs(o, common, merge);
 
 	if (o->detect_directory_renames) {
-		dir_re_head = get_directory_renames(head_pairs, head);
-		dir_re_merge = get_directory_renames(merge_pairs, merge);
+		dir_re_head = get_directory_renames(head_pairs);
+		dir_re_merge = get_directory_renames(merge_pairs);
 
 		handle_directory_level_conflicts(o,
 						 dir_re_head, head,
@@ -3268,8 +3264,7 @@ static int process_entry(struct merge_options *o,
 			clean_merge = 1;
 			if (handle_rename_via_dir(o,
 						  conflict_info->pair1,
-						  conflict_info->branch1,
-						  conflict_info->branch2))
+						  conflict_info->branch1))
 				clean_merge = -1;
 			break;
 		case RENAME_ADD:
-- 
2.21.0.rc1.567.g648f076c3f


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 06/10] pack-objects: drop unused parameter from oe_map_new_pack()
  2019-02-14  5:47 [PATCH 0/10] dropping more unused function parameters Jeff King
                   ` (4 preceding siblings ...)
  2019-02-14  5:50 ` [PATCH 05/10] merge-recursive: drop several unused parameters Jeff King
@ 2019-02-14  5:50 ` Jeff King
  2019-02-14  5:50 ` [PATCH 07/10] files-backend: drop refs parameter from split_symref_update() Jeff King
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Jeff King @ 2019-02-14  5:50 UTC (permalink / raw)
  To: git; +Cc: Duy Nguyen

Since 43fa44fa3b (pack-objects: move in_pack out of struct object_entry,
2018-04-14), we store the source pack for each object as a small index
rather than as a pointer. When we see a new pack that has no allocated
index, we fall back to generating an array of pointers by calling
oe_map_new_pack().

Perhaps counter-intuitively, that function does not need to actually see
our new index-less pack. It only allocates and populates the array with
the existing packs, after which oe_set_in_pack() actually adds the new
pack to the array.

Let's drop the unused "struct packed_git" argument to oe_map_new_pack()
to avoid confusion.

Signed-off-by: Jeff King <peff@peff.net>
---
 pack-objects.c | 3 +--
 pack-objects.h | 6 +++---
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/pack-objects.c b/pack-objects.c
index e7cd337bee..ce33b8906e 100644
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -119,8 +119,7 @@ static void prepare_in_pack_by_idx(struct packing_data *pdata)
  * this fall back code, just stay simple and fall back to using
  * in_pack[] array.
  */
-void oe_map_new_pack(struct packing_data *pack,
-		     struct packed_git *p)
+void oe_map_new_pack(struct packing_data *pack)
 {
 	uint32_t i;
 
diff --git a/pack-objects.h b/pack-objects.h
index 6bfacc7d2c..6fde7ce27c 100644
--- a/pack-objects.h
+++ b/pack-objects.h
@@ -247,14 +247,14 @@ static inline struct packed_git *oe_in_pack(const struct packing_data *pack,
 		return pack->in_pack[e - pack->objects];
 }
 
-void oe_map_new_pack(struct packing_data *pack,
-		     struct packed_git *p);
+void oe_map_new_pack(struct packing_data *pack);
+
 static inline void oe_set_in_pack(struct packing_data *pack,
 				  struct object_entry *e,
 				  struct packed_git *p)
 {
 	if (!p->index)
-		oe_map_new_pack(pack, p);
+		oe_map_new_pack(pack);
 	if (pack->in_pack_by_idx)
 		e->in_pack_idx = p->index;
 	else
-- 
2.21.0.rc1.567.g648f076c3f


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 07/10] files-backend: drop refs parameter from split_symref_update()
  2019-02-14  5:47 [PATCH 0/10] dropping more unused function parameters Jeff King
                   ` (5 preceding siblings ...)
  2019-02-14  5:50 ` [PATCH 06/10] pack-objects: drop unused parameter from oe_map_new_pack() Jeff King
@ 2019-02-14  5:50 ` Jeff King
  2019-02-14  5:50 ` [PATCH 08/10] ref-filter: drop unused buf/sz pairs Jeff King
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Jeff King @ 2019-02-14  5:50 UTC (permalink / raw)
  To: git; +Cc: Michael Haggerty

This parameter was added in fcc42ea0c9 (split_symref_update(): add a
files_ref_store argument, 2016-09-04) without comment, but never used.
The splitting is purely mechanical, and doesn't depend on the particular
ref-store. Let's drop this parameter in the name of simplicity.

Signed-off-by: Jeff King <peff@peff.net>
---
 refs/files-backend.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index dd8abe9185..ef053f716c 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2254,8 +2254,7 @@ static int split_head_update(struct ref_update *update,
  * Note that the new update will itself be subject to splitting when
  * the iteration gets to it.
  */
-static int split_symref_update(struct files_ref_store *refs,
-			       struct ref_update *update,
+static int split_symref_update(struct ref_update *update,
 			       const char *referent,
 			       struct ref_transaction *transaction,
 			       struct string_list *affected_refnames,
@@ -2449,7 +2448,7 @@ static int lock_ref_for_update(struct files_ref_store *refs,
 			 * of processing the split-off update, so we
 			 * don't have to do it here.
 			 */
-			ret = split_symref_update(refs, update,
+			ret = split_symref_update(update,
 						  referent.buf, transaction,
 						  affected_refnames, err);
 			if (ret)
-- 
2.21.0.rc1.567.g648f076c3f


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 08/10] ref-filter: drop unused buf/sz pairs
  2019-02-14  5:47 [PATCH 0/10] dropping more unused function parameters Jeff King
                   ` (6 preceding siblings ...)
  2019-02-14  5:50 ` [PATCH 07/10] files-backend: drop refs parameter from split_symref_update() Jeff King
@ 2019-02-14  5:50 ` Jeff King
  2019-02-14  5:50 ` [PATCH 09/10] ref-filter: drop unused "obj" parameters Jeff King
  2019-02-14  5:51 ` [PATCH 10/10] ref-filter: drop unused "sz" parameters Jeff King
  9 siblings, 0 replies; 11+ messages in thread
From: Jeff King @ 2019-02-14  5:50 UTC (permalink / raw)
  To: git

The grab_tag_values() and grab_commit_values() functions take both the
"struct object" as well as the buf/sz pair for the actual object bytes.
However, neither function uses the latter, as they pull the data
directly from the parsed object struct.

Let's get rid of these misleading parameters.

Signed-off-by: Jeff King <peff@peff.net>
---
 ref-filter.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 422a9c9ae3..c9333b3181 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -913,7 +913,7 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_
 }
 
 /* See grab_values */
-static void grab_tag_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
+static void grab_tag_values(struct atom_value *val, int deref, struct object *obj)
 {
 	int i;
 	struct tag *tag = (struct tag *) obj;
@@ -935,7 +935,7 @@ static void grab_tag_values(struct atom_value *val, int deref, struct object *ob
 }
 
 /* See grab_values */
-static void grab_commit_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
+static void grab_commit_values(struct atom_value *val, int deref, struct object *obj)
 {
 	int i;
 	struct commit *commit = (struct commit *) obj;
@@ -1269,12 +1269,12 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, v
 {
 	switch (obj->type) {
 	case OBJ_TAG:
-		grab_tag_values(val, deref, obj, buf, sz);
+		grab_tag_values(val, deref, obj);
 		grab_sub_body_contents(val, deref, obj, buf, sz);
 		grab_person("tagger", val, deref, obj, buf, sz);
 		break;
 	case OBJ_COMMIT:
-		grab_commit_values(val, deref, obj, buf, sz);
+		grab_commit_values(val, deref, obj);
 		grab_sub_body_contents(val, deref, obj, buf, sz);
 		grab_person("author", val, deref, obj, buf, sz);
 		grab_person("committer", val, deref, obj, buf, sz);
-- 
2.21.0.rc1.567.g648f076c3f


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 09/10] ref-filter: drop unused "obj" parameters
  2019-02-14  5:47 [PATCH 0/10] dropping more unused function parameters Jeff King
                   ` (7 preceding siblings ...)
  2019-02-14  5:50 ` [PATCH 08/10] ref-filter: drop unused buf/sz pairs Jeff King
@ 2019-02-14  5:50 ` Jeff King
  2019-02-14  5:51 ` [PATCH 10/10] ref-filter: drop unused "sz" parameters Jeff King
  9 siblings, 0 replies; 11+ messages in thread
From: Jeff King @ 2019-02-14  5:50 UTC (permalink / raw)
  To: git

The grab_person() and grab_sub_body_contents() functions take both an
object struct and a buf/sz pair of the object bytes. However, they use
only the latter, since "struct object" does not contain the parsed ident
(nor the whole commit message, of course).

Let's get rid of these misleading "struct object" parameters. It's
possible we may want them in the future (e.g., to generate error
messages that mention the object id), but since these are static
functions, we can easily add them back in later (and if we do want that
information, it's likely we'd pass it through a more generalized
"parsing context" struct anyway).

Signed-off-by: Jeff King <peff@peff.net>
---
 ref-filter.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index c9333b3181..9ba18b220c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1064,7 +1064,7 @@ static void grab_date(const char *buf, struct atom_value *v, const char *atomnam
 }
 
 /* See grab_values */
-static void grab_person(const char *who, struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
+static void grab_person(const char *who, struct atom_value *val, int deref, void *buf, unsigned long sz)
 {
 	int i;
 	int wholen = strlen(who);
@@ -1192,7 +1192,7 @@ static void append_lines(struct strbuf *out, const char *buf, unsigned long size
 }
 
 /* See grab_values */
-static void grab_sub_body_contents(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
+static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf, unsigned long sz)
 {
 	int i;
 	const char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;
@@ -1270,14 +1270,14 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, v
 	switch (obj->type) {
 	case OBJ_TAG:
 		grab_tag_values(val, deref, obj);
-		grab_sub_body_contents(val, deref, obj, buf, sz);
-		grab_person("tagger", val, deref, obj, buf, sz);
+		grab_sub_body_contents(val, deref, buf, sz);
+		grab_person("tagger", val, deref, buf, sz);
 		break;
 	case OBJ_COMMIT:
 		grab_commit_values(val, deref, obj);
-		grab_sub_body_contents(val, deref, obj, buf, sz);
-		grab_person("author", val, deref, obj, buf, sz);
-		grab_person("committer", val, deref, obj, buf, sz);
+		grab_sub_body_contents(val, deref, buf, sz);
+		grab_person("author", val, deref, buf, sz);
+		grab_person("committer", val, deref, buf, sz);
 		break;
 	case OBJ_TREE:
 		/* grab_tree_values(val, deref, obj, buf, sz); */
-- 
2.21.0.rc1.567.g648f076c3f


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 10/10] ref-filter: drop unused "sz" parameters
  2019-02-14  5:47 [PATCH 0/10] dropping more unused function parameters Jeff King
                   ` (8 preceding siblings ...)
  2019-02-14  5:50 ` [PATCH 09/10] ref-filter: drop unused "obj" parameters Jeff King
@ 2019-02-14  5:51 ` Jeff King
  9 siblings, 0 replies; 11+ messages in thread
From: Jeff King @ 2019-02-14  5:51 UTC (permalink / raw)
  To: git

Many of our grab_* functions, which parse the object content, take a
buf/sz pair of the object bytes. However, the functions which actually
parse the buffers (like find_wholine() and find_subpos()) never look at
"sz", and instead use functions like strchr() and strchrnul() that
assume the result is NUL-terminated.

This is OK in practice (and common for Git's parsing code), since we
always allocate an extra NUL when loading an object into memory (and
likewise, we are OK with stopping parsing if a commit or tag contains an
embedded NUL).

Let's drop these extra "sz" parameters, as they are misleading about how
the functions intend to access the buffer. We can drop from both the
functions mentioned above, which in turn lets us drop from their
callers, cascading all the way up to the top-level grab_values().

Signed-off-by: Jeff King <peff@peff.net>
---
 ref-filter.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 9ba18b220c..09b5fb439f 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -968,7 +968,7 @@ static void grab_commit_values(struct atom_value *val, int deref, struct object
 	}
 }
 
-static const char *find_wholine(const char *who, int wholen, const char *buf, unsigned long sz)
+static const char *find_wholine(const char *who, int wholen, const char *buf)
 {
 	const char *eol;
 	while (*buf) {
@@ -1064,7 +1064,7 @@ static void grab_date(const char *buf, struct atom_value *v, const char *atomnam
 }
 
 /* See grab_values */
-static void grab_person(const char *who, struct atom_value *val, int deref, void *buf, unsigned long sz)
+static void grab_person(const char *who, struct atom_value *val, int deref, void *buf)
 {
 	int i;
 	int wholen = strlen(who);
@@ -1085,7 +1085,7 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void
 		    !starts_with(name + wholen, "date"))
 			continue;
 		if (!wholine)
-			wholine = find_wholine(who, wholen, buf, sz);
+			wholine = find_wholine(who, wholen, buf);
 		if (!wholine)
 			return; /* no point looking for it */
 		if (name[wholen] == 0)
@@ -1105,7 +1105,7 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void
 	if (strcmp(who, "tagger") && strcmp(who, "committer"))
 		return; /* "author" for commit object is not wanted */
 	if (!wholine)
-		wholine = find_wholine(who, wholen, buf, sz);
+		wholine = find_wholine(who, wholen, buf);
 	if (!wholine)
 		return;
 	for (i = 0; i < used_atom_cnt; i++) {
@@ -1123,7 +1123,7 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void
 	}
 }
 
-static void find_subpos(const char *buf, unsigned long sz,
+static void find_subpos(const char *buf,
 			const char **sub, unsigned long *sublen,
 			const char **body, unsigned long *bodylen,
 			unsigned long *nonsiglen,
@@ -1192,7 +1192,7 @@ static void append_lines(struct strbuf *out, const char *buf, unsigned long size
 }
 
 /* See grab_values */
-static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf, unsigned long sz)
+static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)
 {
 	int i;
 	const char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;
@@ -1212,7 +1212,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf,
 		    !starts_with(name, "contents"))
 			continue;
 		if (!subpos)
-			find_subpos(buf, sz,
+			find_subpos(buf,
 				    &subpos, &sublen,
 				    &bodypos, &bodylen, &nonsiglen,
 				    &sigpos, &siglen);
@@ -1265,19 +1265,19 @@ static void fill_missing_values(struct atom_value *val)
  * pointed at by the ref itself; otherwise it is the object the
  * ref (which is a tag) refers to.
  */
-static void grab_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
+static void grab_values(struct atom_value *val, int deref, struct object *obj, void *buf)
 {
 	switch (obj->type) {
 	case OBJ_TAG:
 		grab_tag_values(val, deref, obj);
-		grab_sub_body_contents(val, deref, buf, sz);
-		grab_person("tagger", val, deref, buf, sz);
+		grab_sub_body_contents(val, deref, buf);
+		grab_person("tagger", val, deref, buf);
 		break;
 	case OBJ_COMMIT:
 		grab_commit_values(val, deref, obj);
-		grab_sub_body_contents(val, deref, buf, sz);
-		grab_person("author", val, deref, buf, sz);
-		grab_person("committer", val, deref, buf, sz);
+		grab_sub_body_contents(val, deref, buf);
+		grab_person("author", val, deref, buf);
+		grab_person("committer", val, deref, buf);
 		break;
 	case OBJ_TREE:
 		/* grab_tree_values(val, deref, obj, buf, sz); */
@@ -1516,7 +1516,7 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj
 			return strbuf_addf_ret(err, -1, _("parse_object_buffer failed on %s for %s"),
 					       oid_to_hex(&oi->oid), ref->refname);
 		}
-		grab_values(ref->value, deref, *obj, oi->content, oi->size);
+		grab_values(ref->value, deref, *obj, oi->content);
 	}
 
 	grab_common_values(ref->value, deref, oi);
-- 
2.21.0.rc1.567.g648f076c3f

^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-02-14  5:56 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-14  5:47 [PATCH 0/10] dropping more unused function parameters Jeff King
2019-02-14  5:48 ` [PATCH 01/10] diff: drop options parameter from diffcore_fix_diff_index() Jeff King
2019-02-14  5:48 ` [PATCH 02/10] diff: drop unused color reset parameters Jeff King
2019-02-14  5:48 ` [PATCH 03/10] diff: drop unused emit data parameter from sane_truncate_line() Jeff King
2019-02-14  5:49 ` [PATCH 04/10] diff: drop complete_rewrite parameter from run_external_diff() Jeff King
2019-02-14  5:50 ` [PATCH 05/10] merge-recursive: drop several unused parameters Jeff King
2019-02-14  5:50 ` [PATCH 06/10] pack-objects: drop unused parameter from oe_map_new_pack() Jeff King
2019-02-14  5:50 ` [PATCH 07/10] files-backend: drop refs parameter from split_symref_update() Jeff King
2019-02-14  5:50 ` [PATCH 08/10] ref-filter: drop unused buf/sz pairs Jeff King
2019-02-14  5:50 ` [PATCH 09/10] ref-filter: drop unused "obj" parameters Jeff King
2019-02-14  5:51 ` [PATCH 10/10] ref-filter: drop unused "sz" parameters Jeff King

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).