Re: [PATCH 1/2] Remove unused variables
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Mar 22, 2011, 19:43 UTC
- Message-ID
- <20110322194309.GA9677@elie>
- In-Reply-To
- <2aeb4f915afaf712c1d5bba46f4010a325ea14c5.1300798069.git.johannes.schindelin@gmx.de>
Hi,
These all look good. Some notes:
Johannes Schindelin wrote:
Show 8 quoted lines
> --- 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.
Show 8 quoted lines
> --- 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.
Show 8 quoted lines
> --- 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.
Show 7 quoted lines
> --- 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).
Show 5 quoted lines
> @@ -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.
Show 19 quoted lines
> --- 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.
Show 7 quoted lines
> --- 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