[PATCH] revision: remove definition of unused 'add_object' function

Ramsay Jones· Oct 18, 2014, 21:36 UTC · lore

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---

Hi Jeff,

I noticed that your 'jk/prune-mtime' branch removes the last caller of the add_object() function; specifically commit 5f78a431a ("reachable: use traverse_commit_list instead of custom walk", 15-10-2014).

If you need to re-roll those patches, could you please squash this patch into the above commit. (unless you have plans to add some new callers, of course! ;-) ).

Thanks!

ATB, Ramsay Jones

 revision.c | 10 ----------
 revision.h |  5 -----
 2 files changed, 15 deletions(-)

Show changes to 2 files +0 −15

revision.c, revision.h

diff --git a/revision.c b/revision.c
index 1316fe0..7bdf760 100644
--- a/revision.c
+++ b/revision.c
@@ -87,16 +87,6 @@ void show_object_with_name(FILE *out, struct object *obj,
 	fputc('\n', out);
 }
 
-void add_object(struct object *obj,
-		struct object_array *p,
-		struct name_path *path,
-		const char *name)
-{
-	char *pn = path_name(path, name);
-	add_object_array(obj, pn, p);
-	free(pn);
-}
-
 static void mark_blob_uninteresting(struct blob *blob)
 {
 	if (!blob)
diff --git a/revision.h b/revision.h
index 9ab6755..b90f907 100644
--- a/revision.h
+++ b/revision.h
@@ -282,11 +282,6 @@ char *path_name(const struct name_path *path, const char *name);
 extern void show_object_with_name(FILE *, struct object *,
 				  const struct name_path *, const char *);
 
-extern void add_object(struct object *obj,
-		       struct object_array *p,
-		       struct name_path *path,
-		       const char *name);
-
 extern void add_pending_object(struct rev_info *revs,
 			       struct object *obj, const char *name);
 extern void add_pending_sha1(struct rev_info *revs,

-- 
2.1.0

Jeff King· Oct 19, 2014, 01:36 UTC · re: Ramsay Jones · lore

Re: [PATCH] revision: remove definition of unused 'add_object' function

On Sat, Oct 18, 2014 at 10:36:12PM +0100, Ramsay Jones wrote:

> I noticed that your 'jk/prune-mtime' branch removes the last caller
> of the add_object() function; specifically commit 5f78a431a
> ("reachable: use traverse_commit_list instead of custom walk", 15-10-2014).

Thanks. I usually rely on the compiler to catch any static instances that I missed, but of course this one is extern. Did you use an automated tool for this (I know you often catch "X has no declaration; should it be static?" with clang, but does clang catch this, too?).

> If you need to re-roll those patches, could you please squash this
> patch into the above commit. (unless you have plans to add some new
> callers, of course! ;-) ).

Nope, I just didn't notice that I dropped the last caller. I don't think we need another re-roll (fingers crossed), but I'd be happy to have this on top.

-Peff

Ramsay Jones· Oct 19, 2014, 10:16 UTC · re: Jeff King · lore

Re: [PATCH] revision: remove definition of unused 'add_object' function

On 19/10/14 02:36, Jeff King wrote:

Show 9 quoted lines

> On Sat, Oct 18, 2014 at 10:36:12PM +0100, Ramsay Jones wrote:
> 
>> I noticed that your 'jk/prune-mtime' branch removes the last caller
>> of the add_object() function; specifically commit 5f78a431a
>> ("reachable: use traverse_commit_list instead of custom walk", 15-10-2014).
> 
> Thanks. I usually rely on the compiler to catch any static instances
> that I missed, but of course this one is extern. Did you use an
> automated tool for this

Yes, see below.

>                         (I know you often catch "X has no declaration;
> should it be static?" with clang,

No, these are caught with sparse.

Show 9 quoted lines

>                                    but does clang catch this, too?).
> 
>> If you need to re-roll those patches, could you please squash this
>> patch into the above commit. (unless you have plans to add some new
>> callers, of course! ;-) ).
> 
> Nope, I just didn't notice that I dropped the last caller. I don't think
> we need another re-roll (fingers crossed), but I'd be happy to have this
> on top.

OK, Thanks!

The tool that I use to spot these situations is actually my evolution of a perl script that Junio sent to the list ages ago! ;-)

I made only minor alterations so that it would work on MinGW 32-bit (well, actually msysGit, to be more precise), Cygwin 32-bit and 64-bit. (The 'stop list' of acceptable symbols is _way_ out of date, but the way I use the script that doesn't matter; still its on my TODO).

I run the script over each branch I build, master then next then pu, and diff the results (ie ignoring the master base symbols), so I generally only notice new symbols introduced into pu by new topics. It has been on my TODO list for a while (OK years!) to go through the output from the master branch and remove/make static those symbols which shouldn't be external. (There are currently 53 symbols on 64-bit Linux. The list is not identical on every platform, so you need to be careful. Also cgit uses some symbols which would otherwise be static in git! ;-) ).

Ahem, I don't use git to version the script (static-check.pl), so I'm only reasonable confident that the script I have easily to hand (Linux Mint 64-bit) is the same on all platforms ... included below.

ATB, Ramsay Jones

-- >8 -- #!/usr/bin/perl -w

my %defd = ();
my %used = ();
my %def_ok = map { $_ => 1 } qw(
	main
	alloc_report
	have_git_dir
	prepare_git_cmd
	print_string_list
	tm_to_time_t
	unsorted_string_list_has_string
	xdl_atol
	xdl_cha_first
	xdl_cha_next
	xdl_mmfile_size
	xdl_num_out
	xdl_recs_cmp
);

for (<*.o>, <*/*.o>, <*/*/*.o>) {
	my $obj = $_;
	open(I, "-|", qw(nm -gC), $obj) or die;
	while (<I>) {
		unless (/^[0-9a-f ]+([A-Z]) (\S*)$/) {
			print STDERR "? $_";
			next;
		}
		next if ($2 =~ /^\.refptr\./);
		if (($1 eq "U") || $1 eq "C") {
			$used{$2}++;
		}
		else {
			push @{$defd{$obj}}, $2;
		}
	}
	close I;
}

for my $obj (sort keys %defd) {
	my $syms = $defd{$obj};
	for my $sym (@$syms) {
		next if exists $used{$sym} or exists $def_ok{$sym};
		print "$obj	- $sym\n";
	}
}

patchrevision: remove definition of unused 'add_object' function