Re: [PATCH] git-credential-netrc: fix uninitialized warning

From: Ted Zlatanov <tzz@lifelogs.com>
Date: Oct 8, 2013, 19:55 UTC
Message-ID: <87li23v8p5.fsf@flea.lifelogs.com>
In-Reply-To: <20131008194147.GF9464@google.com>

On Tue, 8 Oct 2013 12:41:47 -0700 Jonathan Nieder <jrnieder@gmail.com> wrote:

JN> Ted Zlatanov wrote:

>> Simple patch to avoid unitialized warning and log what we'll do.

JN> Sign-off?

I didn't realize it was a requirement, must I?

JN> [...]

Show 11 quoted lines

>> --- a/contrib/credential/netrc/git-credential-netrc
>> +++ b/contrib/credential/netrc/git-credential-netrc
>> @@ -369,7 +369,10 @@ sub find_netrc_entry {
>> {
>> my $entry_text = join ', ', map { "$_=$entry->{$_}" } keys %$entry;
>> foreach my $check (sort keys %$query) {
>> -			if (defined $query->{$check}) {
>> +			if (!defined $entry->{$check}) {
>> +			       log_debug("OK: entry has no $check token, so any value satisfies check $check");
>> +			}
>> +			elsif (defined $query->{$check}) {

JN> Style: elsewhere this file seems to use cuddled elses:

JN> 	} elsif (...) {

Ah, thanks, I missed that.

JN> Or more simply, would it make sense to wrap both 'defined' checks into JN> a single "if", like so?

JN> if (defined $entry->{$check} && defined $query->{$check}) { JN> ... JN> } else { JN> log_debug(...); JN> }

I prefer the explicit version because we can issue a more precise log_debug message.

Ted