Skip to content

Commit

Permalink
git-svn: Always duplicate paths returned from get_log
Browse files Browse the repository at this point in the history
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 <mattias.nissler@gmx.de>
Acked-by: Eric Wong <normalperson@yhbt.net>
  • Loading branch information
saittam authored and Eric Wong committed Jul 11, 2009
1 parent d9eb020 commit 3c49a03
Showing 1 changed file with 23 additions and 22 deletions.
45 changes: 23 additions & 22 deletions git-svn.perl
Original file line number Diff line number Diff line change
Expand Up @@ -2538,9 +2538,8 @@ sub find_parent_branch {
unless (defined $paths) {
my $err_handler = $SVN::Error::handler;
$SVN::Error::handler = \&Git::SVN::Ra::skip_unknown_revs;
$self->ra->get_log([$self->{path}], $rev, $rev, 0, 1, 1, sub {
$paths =
Git::SVN::Ra::dup_changed_paths($_[0]) });
$self->ra->get_log([$self->{path}], $rev, $rev, 0, 1, 1,
sub { $paths = $_[0] });
$SVN::Error::handler = $err_handler;
}
return undef unless defined $paths;
Expand Down Expand Up @@ -4431,6 +4430,26 @@ sub get_log {
my ($self, @args) = @_;
my $pool = SVN::Pool->new;

# svn_log_changed_path_t objects passed to get_log are likely to be
# overwritten even if only the refs are copied to an external variable,
# so we should dup the structures in their entirety. Using an
# externally passed pool (instead of our temporary and quickly cleared
# pool in Git::SVN::Ra) does not help matters at all...
my $receiver = pop @args;
push(@args, sub {
my ($paths) = $_[0];
return &$receiver(@_) unless $paths;
$_[0] = ();
foreach my $p (keys %$paths) {
my $i = $paths->{$p};
my %s = map { $_ => $i->$_ }
qw/copyfrom_path copyfrom_rev action/;
$_[0]{$p} = \%s;
}
&$receiver(@_);
});


# the limit parameter was not supported in SVN 1.1.x, so we
# drop it. Therefore, the receiver callback passed to it
# is made aware of this limitation by being wrapped if
Expand Down Expand Up @@ -4600,7 +4619,7 @@ sub gs_fetch_loop_common {
};
sub _cb {
my ($paths, $r, $author, $date, $log) = @_;
[ dup_changed_paths($paths),
[ $paths,
{ author => $author, date => $date, log => $log } ];
}
$self->get_log([$longest_path], $min, $max, 0, 1, 1,
Expand Down Expand Up @@ -4823,24 +4842,6 @@ sub skip_unknown_revs {
die "Error from SVN, ($errno): ", $err->expanded_message,"\n";
}

# svn_log_changed_path_t objects passed to get_log are likely to be
# overwritten even if only the refs are copied to an external variable,
# so we should dup the structures in their entirety. Using an externally
# passed pool (instead of our temporary and quickly cleared pool in
# Git::SVN::Ra) does not help matters at all...
sub dup_changed_paths {
my ($paths) = @_;
return undef unless $paths;
my %ret;
foreach my $p (keys %$paths) {
my $i = $paths->{$p};
my %s = map { $_ => $i->$_ }
qw/copyfrom_path copyfrom_rev action/;
$ret{$p} = \%s;
}
\%ret;
}

package Git::SVN::Log;
use strict;
use warnings;
Expand Down

0 comments on commit 3c49a03

Please sign in to comment.