Re: [PATCH 1/2] Remove unused variables

[Jonathan Nieder <jrnieder@gmail.com>]

Hi,

These all look good.  Some notes:

Johannes Schindelin wrote:

> --- a/builtin/fsck.c
> +++ b/builtin/fsck.c
> @@ -140,11 +140,10 @@ static int traverse_reachable(void)
>  	int result = 0;
>  	while (pending.nr) {
>  		struct object_array_entry *entry;
> -		struct object *obj, *parent;
> +		struct object *obj;

Cleaning up after v1.7.4.1~1^2~1 (fsck: drop unused parameter from
traverse_one_object(), 2011-01-26).  Sensible.

> --- a/builtin/remote-ext.c
> +++ b/builtin/remote-ext.c
> @@ -30,16 +30,12 @@ static char *strip_escapes(const char *str, const char *service,
>  	size_t rpos = 0;
>  	int escape = 0;
>  	char special = 0;
> -	size_t pslen = 0;
> -	size_t pSlen = 0;

These were never used; sorry to have missed it before.

> --- a/diff.c
> +++ b/diff.c
> @@ -1242,7 +1242,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
>  	uintmax_t max_change = 0, max_len = 0;
>  	int total_files = data->nr;
>  	int width, name_width;
> -	const char *reset, *set, *add_c, *del_c;
> +	const char *reset, *add_c, *del_c;

After v1.6.4-rc0~159^2 (diff: do not color --stat output like patch
context, 2009-04-25), the "patch context" color is not used in
diffstats.

> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -357,7 +357,6 @@ static void make_room_for_directories_of_df_conflicts(struct merge_options *o,
>  	 */
>  	const char *last_file = NULL;
>  	int last_len = 0;
> -	struct stage_data *last_e;

Yep, was never used (and while the code to set it is correct, it
is better to remove it than to let it bitrot).

> @@ -969,7 +967,6 @@ static int process_renames(struct merge_options *o,
>  	}
>  
>  	for (i = 0, j = 0; i < a_renames->nr || j < b_renames->nr;) {
> -		char *src;

Likewise.

> --- a/reachable.c
> +++ b/reachable.c
> @@ -70,16 +70,11 @@ static void process_tree(struct tree *tree,
>  static void process_tag(struct tag *tag, struct object_array *p, const char *name)
>  {
>  	struct object *obj = &tag->object;
> -	struct name_path me;
>  
>  	if (obj->flags & SEEN)
>  		return;
>  	obj->flags |= SEEN;
>  
> -	me.up = NULL;
> -	me.elem = "tag:/";
> -	me.elem_len = 5;
> -
>  	if (parse_tag(tag) < 0)
>  		die("bad tag object %s", sha1_to_hex(obj->sha1));
>  	if (tag->tagged)

Does this code make sense at all?  It peels a tag once in order to
mark the object it refers to with "add_object", which adds the object
to the array referred to by "p", that nobody actually uses.  Maybe
this is a remnant of an earlier implementation for "git log
--objects"?

Peeling of tags is already taken care of by revision::handle_commit,
so I think the function could be stripped down to

 static void process_tag(struct tag *tag, struct object_array *p, const char *name)
 {
	struct object *obj = &tag->object;
 
	if (obj->flags & SEEN)
		return;
	obj->flags |= SEEN;
 }

which seems to work okay still.

> --- a/test-subprocess.c
> +++ b/test-subprocess.c
> @@ -3,11 +3,10 @@
[...]
> -	prefix = setup_git_directory_gently(&nogit);
> +	setup_git_directory_gently(&nogit);

It might be more idiomatic to use setup_git_directory, but this is
better because conservative.

> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -561,10 +561,9 @@ static int push_refs_with_push(struct transport *transport,
>  	int mirror = flags & TRANSPORT_PUSH_MIRROR;
>  	struct helper_data *data = transport->data;
>  	struct strbuf buf = STRBUF_INIT;
> -	struct child_process *helper;

Cleaning up after v1.7.0-rc0~62^2~12 (Add remote helper debug mode,
2009-12-09).  The remote helper process is accessed through
transport->data->helper.

Thanks and hope that helps.
Jonathan