Skip to content

Commit 3c49a03

Browse files
saittamEric Wong
authored andcommitted
git-svn: Always duplicate paths returned from get_log
This makes get_log more safe to use because callers cannot run into path clobbering any more. The additional overhead will not affect performance since the critical calls from the fetch loop need the path duplication anyway and the rest of the call sites is not performance critical. Signed-off-by: Mattias Nissler <[email protected]> Acked-by: Eric Wong <[email protected]>
1 parent d9eb020 commit 3c49a03

File tree

1 file changed

+23
-22
lines changed

1 file changed

+23
-22
lines changed

git-svn.perl

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2538,9 +2538,8 @@ sub find_parent_branch {
25382538
unless (defined $paths) {
25392539
my $err_handler = $SVN::Error::handler;
25402540
$SVN::Error::handler = \&Git::SVN::Ra::skip_unknown_revs;
2541-
$self->ra->get_log([$self->{path}], $rev, $rev, 0, 1, 1, sub {
2542-
$paths =
2543-
Git::SVN::Ra::dup_changed_paths($_[0]) });
2541+
$self->ra->get_log([$self->{path}], $rev, $rev, 0, 1, 1,
2542+
sub { $paths = $_[0] });
25442543
$SVN::Error::handler = $err_handler;
25452544
}
25462545
return undef unless defined $paths;
@@ -4431,6 +4430,26 @@ sub get_log {
44314430
my ($self, @args) = @_;
44324431
my $pool = SVN::Pool->new;
44334432

4433+
# svn_log_changed_path_t objects passed to get_log are likely to be
4434+
# overwritten even if only the refs are copied to an external variable,
4435+
# so we should dup the structures in their entirety. Using an
4436+
# externally passed pool (instead of our temporary and quickly cleared
4437+
# pool in Git::SVN::Ra) does not help matters at all...
4438+
my $receiver = pop @args;
4439+
push(@args, sub {
4440+
my ($paths) = $_[0];
4441+
return &$receiver(@_) unless $paths;
4442+
$_[0] = ();
4443+
foreach my $p (keys %$paths) {
4444+
my $i = $paths->{$p};
4445+
my %s = map { $_ => $i->$_ }
4446+
qw/copyfrom_path copyfrom_rev action/;
4447+
$_[0]{$p} = \%s;
4448+
}
4449+
&$receiver(@_);
4450+
});
4451+
4452+
44344453
# the limit parameter was not supported in SVN 1.1.x, so we
44354454
# drop it. Therefore, the receiver callback passed to it
44364455
# is made aware of this limitation by being wrapped if
@@ -4600,7 +4619,7 @@ sub gs_fetch_loop_common {
46004619
};
46014620
sub _cb {
46024621
my ($paths, $r, $author, $date, $log) = @_;
4603-
[ dup_changed_paths($paths),
4622+
[ $paths,
46044623
{ author => $author, date => $date, log => $log } ];
46054624
}
46064625
$self->get_log([$longest_path], $min, $max, 0, 1, 1,
@@ -4823,24 +4842,6 @@ sub skip_unknown_revs {
48234842
die "Error from SVN, ($errno): ", $err->expanded_message,"\n";
48244843
}
48254844

4826-
# svn_log_changed_path_t objects passed to get_log are likely to be
4827-
# overwritten even if only the refs are copied to an external variable,
4828-
# so we should dup the structures in their entirety. Using an externally
4829-
# passed pool (instead of our temporary and quickly cleared pool in
4830-
# Git::SVN::Ra) does not help matters at all...
4831-
sub dup_changed_paths {
4832-
my ($paths) = @_;
4833-
return undef unless $paths;
4834-
my %ret;
4835-
foreach my $p (keys %$paths) {
4836-
my $i = $paths->{$p};
4837-
my %s = map { $_ => $i->$_ }
4838-
qw/copyfrom_path copyfrom_rev action/;
4839-
$ret{$p} = \%s;
4840-
}
4841-
\%ret;
4842-
}
4843-
48444845
package Git::SVN::Log;
48454846
use strict;
48464847
use warnings;

0 commit comments

Comments
 (0)