{"thread":{"id":"8606","subject":"[PATCH] git-svn: avoid string eval for defining functions","startedAt":"2007-06-15T03:43:59Z","lastAt":"2007-06-16T00:22:54Z","messageCount":2,"participants":["Sam Vilain","Eric Wong"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"45109","messageId":"11818790393274-git-send-email-sam.vilain@catalyst.net.nz","threadId":"8606","inReplyTo":null,"subject":"[PATCH] git-svn: avoid string eval for defining functions","fromName":"Sam Vilain","fromEmail":"sam.vilain@catalyst.net.nz","sentAt":"2007-06-15T03:43:59Z","receivedAt":"2007-06-15T03:43:59Z","isPatch":true,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"You don't need to use string eval to define new functions; assigning a\ncode reference to the target symbol table is enough.\n---\n git-svn.perl |   64 +++++++++++++++++++++++++++++----------------------------\n 1 files changed, 33 insertions(+), 31 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 4d35895..4ba0813 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -38,14 +38,16 @@ use IPC::Open3;\n use Git;\n \n BEGIN {\n-\tmy $s;\n+\t# import functions from Git into our packages, en masse\n+\tno strict 'refs';\n \tforeach (qw/command command_oneline command_noisy command_output_pipe\n \t            command_input_pipe command_close_pipe/) {\n-\t\t$s .= \"*SVN::Git::Editor::$_ = *SVN::Git::Fetcher::$_ = \".\n-\t\t      \"*Git::SVN::Migration::$_ = \".\n-\t\t      \"*Git::SVN::Log::$_ = *Git::SVN::$_ = *$_ = *Git::$_; \";\n+\t\tfor my $package ( qw(SVN::Git::Editor SVN::Git::Fetcher\n+\t\t\tGit::SVN::Migration Git::SVN::Log Git::SVN),\n+\t\t\t__PACKAGE__) {\n+\t\t\t*{\"${package}::$_\"} = \\&{\"Git::$_\"};\n+\t\t}\n \t}\n-\teval $s;\n }\n \n my ($SVN);\n@@ -898,26 +900,26 @@ BEGIN {\n \t# some options are read globally, but can be overridden locally\n \t# per [svn-remote \"...\"] section.  Command-line options will *NOT*\n \t# override options set in an [svn-remote \"...\"] section\n-\tmy $e;\n-\tforeach (qw/follow_parent no_metadata use_svm_props\n-\t            use_svnsync_props/) {\n-\t\tmy $key = $_;\n+\tno strict 'refs';\n+\tfor my $option (qw/follow_parent no_metadata use_svm_props\n+\t\t\t   use_svnsync_props/) {\n+\t\tmy $key = $option;\n \t\t$key =~ tr/_//d;\n-\t\t$e .= \"sub $_ {\n-\t\t\tmy (\\$self) = \\@_;\n-\t\t\treturn \\$self->{-$_} if exists \\$self->{-$_};\n-\t\t\tmy \\$k = \\\"svn-remote.\\$self->{repo_id}\\.$key\\\";\n-\t\t\teval { command_oneline(qw/config --get/, \\$k) };\n-\t\t\tif (\\$@) {\n-\t\t\t\t\\$self->{-$_} = \\$Git::SVN::_$_;\n+\t\tmy $prop = \"-$option\";\n+\t\t*$option = sub {\n+\t\t\tmy ($self) = @_;\n+\t\t\treturn $self->{$prop} if exists $self->{$prop};\n+\t\t\tmy $k = \"svn-remote.$self->{repo_id}.$key\";\n+\t\t\teval { command_oneline(qw/config --get/, $k) };\n+\t\t\tif ($@) {\n+\t\t\t\t$self->{$prop} = ${\"Git::SVN::_$option\"};\n \t\t\t} else {\n-\t\t\t\tmy \\$v = command_oneline(qw/config --bool/,\\$k);\n-\t\t\t\t\\$self->{-$_} = \\$v eq 'false' ? 0 : 1;\n+\t\t\t\tmy $v = command_oneline(qw/config --bool/,$k);\n+\t\t\t\t$self->{$prop} = $v eq 'false' ? 0 : 1;\n \t\t\t}\n-\t\t\treturn \\$self->{-$_} }\\n\";\n+\t\t\treturn $self->{$prop};\n+\t\t}\n \t}\n-\t$e .= \"1;\\n\";\n-\teval $e or die $@;\n }\n \n my %LOCKFILES;\n@@ -2956,17 +2958,17 @@ my ($can_do_switch, %ignored_err, $RA);\n \n BEGIN {\n \t# enforce temporary pool usage for some simple functions\n-\tmy $e;\n-\tforeach (qw/rev_proplist get_latest_revnum get_uuid get_repos_root/) {\n-\t\t$e .= \"sub $_ {\n-\t\t\tmy \\$self = shift;\n-\t\t\tmy \\$pool = SVN::Pool->new;\n-\t\t\tmy \\@ret = \\$self->SUPER::$_(\\@_,\\$pool);\n-\t\t\t\\$pool->clear;\n-\t\t\twantarray ? \\@ret : \\$ret[0]; }\\n\";\n+\tno strict 'refs';\n+\tfor my $f (qw/rev_proplist get_latest_revnum get_uuid get_repos_root/) {\n+\t\tmy $SUPER = \"SUPER::$f\";\n+\t\t*$f = sub {\n+\t\t\tmy $self = shift;\n+\t\t\tmy $pool = SVN::Pool->new;\n+\t\t\tmy @ret = $self->$SUPER(@_,$pool);\n+\t\t\t$pool->clear;\n+\t\t\twantarray ? @ret : $ret[0];\n+\t\t};\n \t}\n-\n-\teval \"$e; 1;\" or die $@;\n }\n \n sub new {\n-- \n1.5.2.0.45.gfea6d-dirty\n"},{"id":"45154","messageId":"20070616002254.GB26502@untitled","threadId":"8606","inReplyTo":"11818790393274-git-send-email-sam.vilain@catalyst.net.nz","subject":"Re: [PATCH] git-svn: avoid string eval for defining functions","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2007-06-16T00:22:54Z","receivedAt":"2007-06-16T00:22:54Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Sam Vilain <sam.vilain@catalyst.net.nz> wrote:\n> You don't need to use string eval to define new functions; assigning a\n> code reference to the target symbol table is enough.\n\nCool.\n\nAcked-by: Eric Wong <normalperson@yhbt.net>\n\n> ---\n>  git-svn.perl |   64 +++++++++++++++++++++++++++++----------------------------\n>  1 files changed, 33 insertions(+), 31 deletions(-)\n> \n> diff --git a/git-svn.perl b/git-svn.perl\n> index 4d35895..4ba0813 100755\n> --- a/git-svn.perl\n> +++ b/git-svn.perl\n> @@ -38,14 +38,16 @@ use IPC::Open3;\n>  use Git;\n>  \n>  BEGIN {\n> -\tmy $s;\n> +\t# import functions from Git into our packages, en masse\n> +\tno strict 'refs';\n>  \tforeach (qw/command command_oneline command_noisy command_output_pipe\n>  \t            command_input_pipe command_close_pipe/) {\n> -\t\t$s .= \"*SVN::Git::Editor::$_ = *SVN::Git::Fetcher::$_ = \".\n> -\t\t      \"*Git::SVN::Migration::$_ = \".\n> -\t\t      \"*Git::SVN::Log::$_ = *Git::SVN::$_ = *$_ = *Git::$_; \";\n> +\t\tfor my $package ( qw(SVN::Git::Editor SVN::Git::Fetcher\n> +\t\t\tGit::SVN::Migration Git::SVN::Log Git::SVN),\n> +\t\t\t__PACKAGE__) {\n> +\t\t\t*{\"${package}::$_\"} = \\&{\"Git::$_\"};\n> +\t\t}\n>  \t}\n> -\teval $s;\n>  }\n>  \n>  my ($SVN);\n> @@ -898,26 +900,26 @@ BEGIN {\n>  \t# some options are read globally, but can be overridden locally\n>  \t# per [svn-remote \"...\"] section.  Command-line options will *NOT*\n>  \t# override options set in an [svn-remote \"...\"] section\n> -\tmy $e;\n> -\tforeach (qw/follow_parent no_metadata use_svm_props\n> -\t            use_svnsync_props/) {\n> -\t\tmy $key = $_;\n> +\tno strict 'refs';\n> +\tfor my $option (qw/follow_parent no_metadata use_svm_props\n> +\t\t\t   use_svnsync_props/) {\n> +\t\tmy $key = $option;\n>  \t\t$key =~ tr/_//d;\n> -\t\t$e .= \"sub $_ {\n> -\t\t\tmy (\\$self) = \\@_;\n> -\t\t\treturn \\$self->{-$_} if exists \\$self->{-$_};\n> -\t\t\tmy \\$k = \\\"svn-remote.\\$self->{repo_id}\\.$key\\\";\n> -\t\t\teval { command_oneline(qw/config --get/, \\$k) };\n> -\t\t\tif (\\$@) {\n> -\t\t\t\t\\$self->{-$_} = \\$Git::SVN::_$_;\n> +\t\tmy $prop = \"-$option\";\n> +\t\t*$option = sub {\n> +\t\t\tmy ($self) = @_;\n> +\t\t\treturn $self->{$prop} if exists $self->{$prop};\n> +\t\t\tmy $k = \"svn-remote.$self->{repo_id}.$key\";\n> +\t\t\teval { command_oneline(qw/config --get/, $k) };\n> +\t\t\tif ($@) {\n> +\t\t\t\t$self->{$prop} = ${\"Git::SVN::_$option\"};\n>  \t\t\t} else {\n> -\t\t\t\tmy \\$v = command_oneline(qw/config --bool/,\\$k);\n> -\t\t\t\t\\$self->{-$_} = \\$v eq 'false' ? 0 : 1;\n> +\t\t\t\tmy $v = command_oneline(qw/config --bool/,$k);\n> +\t\t\t\t$self->{$prop} = $v eq 'false' ? 0 : 1;\n>  \t\t\t}\n> -\t\t\treturn \\$self->{-$_} }\\n\";\n> +\t\t\treturn $self->{$prop};\n> +\t\t}\n>  \t}\n> -\t$e .= \"1;\\n\";\n> -\teval $e or die $@;\n>  }\n>  \n>  my %LOCKFILES;\n> @@ -2956,17 +2958,17 @@ my ($can_do_switch, %ignored_err, $RA);\n>  \n>  BEGIN {\n>  \t# enforce temporary pool usage for some simple functions\n> -\tmy $e;\n> -\tforeach (qw/rev_proplist get_latest_revnum get_uuid get_repos_root/) {\n> -\t\t$e .= \"sub $_ {\n> -\t\t\tmy \\$self = shift;\n> -\t\t\tmy \\$pool = SVN::Pool->new;\n> -\t\t\tmy \\@ret = \\$self->SUPER::$_(\\@_,\\$pool);\n> -\t\t\t\\$pool->clear;\n> -\t\t\twantarray ? \\@ret : \\$ret[0]; }\\n\";\n> +\tno strict 'refs';\n> +\tfor my $f (qw/rev_proplist get_latest_revnum get_uuid get_repos_root/) {\n> +\t\tmy $SUPER = \"SUPER::$f\";\n> +\t\t*$f = sub {\n> +\t\t\tmy $self = shift;\n> +\t\t\tmy $pool = SVN::Pool->new;\n> +\t\t\tmy @ret = $self->$SUPER(@_,$pool);\n> +\t\t\t$pool->clear;\n> +\t\t\twantarray ? @ret : $ret[0];\n> +\t\t};\n>  \t}\n> -\n> -\teval \"$e; 1;\" or die $@;\n>  }\n>  \n>  sub new {\n> -- \n> 1.5.2.0.45.gfea6d-dirty\n> \n\n-- \nEric Wong\n"}]}