On Wed, Feb 06, 2013 at 09:47:05PM +0100, Michal Nazarewicz wrote:
> +sub _credential_read {
> + my %credential;
> + my ($reader, $op) = (@_);
> + while (<$reader>) {
> + chomp;
> + my ($key, $value) = /([^=]*)=(.*)/;
Empty keys are not valid. Can we make this:
/^([^=]+)=(.*)/
to fail the regex? Otherwise, I think this check:
> + if (not defined $key) {
> + throw Error::Simple("unable to parse git credential $op
> response:\n$_\n");
> + }
would not pass because $key would be the empty string.
> +sub _credential_write {
> + my ($credential, $writer) = @_;
> +
> + for my $key (sort {
> + # url overwrites other fields, so it must come first
> + return -1 if $a eq 'url';
> + return 1 if $b eq 'url';
> + return $a cmp $b;
> + } keys %$credential) {
> + if (defined $credential->{$key} && length $credential->{$key}) {
> + print $writer $key, '=', $credential->{$key}, "\n";
> + }
> + }
There are a few disallowed characters, like "\n" in key or value, and
"=" in a key. They should never happen unless the caller is buggy, but
should we check and catch them here?
> +In the second form, C<CODE> needs to be a reference to a subroutine.
> +The function will execute C<git credential fill> to fill provided
> +credential hash, than call C<CODE> with C<CREDENTIAL> as the sole
> +argument, and finally depending on C<CODE>'s return value execute
> +C<git credential approve> (if return value yields true) or C<git
> +credential reject> (otherwise). The return value is the same as what
> +C<CODE> returned. With this form, the usage might look as follows:
This is a nice touch. It makes the normal calling code a lot simpler.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
