From: Ramsay Jones <ramsay@ramsay1•demon.co.uk>
To: Jeff King <peff@peff•net>
Cc: Junio C Hamano <gitster@pobox•com>,
GIT Mailing-list <git@vger•kernel.org>
Subject: Re: [PATCH] revision: remove definition of unused 'add_object' function
Date: Sun, 19 Oct 2014 11:16:00 +0100 [thread overview]
Message-ID: <54438F60.1020500@ramsay1.demon.co.uk> (raw)
In-Reply-To: <20141019013646.GA17908@peff.net>
On 19/10/14 02:36, Jeff King wrote:
> 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.
> 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";
}
}
prev parent reply other threads:[~2014-10-19 10:16 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-18 21:36 [PATCH] revision: remove definition of unused 'add_object' function Ramsay Jones
2014-10-19 1:36 ` Jeff King
2014-10-19 10:16 ` Ramsay Jones [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=54438F60.1020500@ramsay1.demon.co.uk \
--to=ramsay@ramsay1$(echo .)demon.co.uk \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=peff@peff$(echo .)net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox