Thomas Adam <thomas@xxxxxxxxxx> writes:
> On 27 December 2011 20:47, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> Sven Strickroth <sven.strickroth@xxxxxxxxxxxxxxx> writes:
>>> +sub askpass_prompt {
>>> + my ($self, $prompt) = _maybe_self(@_);
>>> + if (exists $ENV{'GIT_ASKPASS'}) {
>>> + return _askpass_prompt($ENV{'GIT_ASKPASS'}, $prompt);
>>> + } elsif (exists $ENV{'SSH_ASKPASS'}) {
>>> + return _askpass_prompt($ENV{'SSH_ASKPASS'}, $prompt);
>>> + } else {
>>> + return undef;
>>
>> Two problems with this if/elsif/else cascade.
>>
>> - If _askpass_prompt() fails to open the pipe to ENV{'GIT_ASKPASS'}, it
>> will return 'undef' to us. Don't we want to fall back to SSH_ASKPASS in
>> such a case?
>>
>> - The last "return undef" makes all callers of this method to implement a
>> fall-back way somehow. I find it very likely that they will want to use
>
> Not only that, "return undef" will have nasty side-effects if this
> subroutine is called in list-context -- it's usually discouraged to
> have explicit returns of "undef", where in scalar context that might
> be OK, but in list context, the caller will see:
>
> (undef)
>
> and not:
>
> ()
>
> i.e., the empty list.
Well, for this particular function whose interface is "I'll give you a
prompt, use it to interact with the user and give me what the user gave us
in response", a scalar caller would do
my $response = askpass_prompt("What is your password?");
while a list context caller would instead do
my ($response) = askpass_prompt("What is your password?");
or
my @answer = askpass_prompt("What is your password?");
my $response = $answer[0];
and all three callers would get "undef" in $response. I suspect returning
(undef) is a better thing to do, than relying that
my @answer = ();
my $response = $answer[0];
happes to give undef to $response because the access goes beyond the end
of the array, no?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html