{"thread":{"id":"35083","subject":"[PATCH] git-credential-netrc: fix uninitialized warning","startedAt":"2013-10-08T14:34:03Z","lastAt":"2013-10-08T20:12:56Z","messageCount":7,"participants":["Ted Zlatanov","Jonathan Nieder","Stefan Beller"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"228611","messageId":"87zjqjx25g.fsf@flea.lifelogs.com","threadId":"35083","inReplyTo":null,"subject":"[PATCH] git-credential-netrc: fix uninitialized warning","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-10-08T14:34:03Z","receivedAt":"2013-10-08T14:34:03Z","isPatch":true,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"Simple patch to avoid unitialized warning and log what we'll do.\n\n---\n contrib/credential/netrc/git-credential-netrc | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc\nindex 6c51c43..13e537b 100755\n--- a/contrib/credential/netrc/git-credential-netrc\n+++ b/contrib/credential/netrc/git-credential-netrc\n@@ -369,7 +369,10 @@ sub find_netrc_entry {\n \t{\n \t\tmy $entry_text = join ', ', map { \"$_=$entry->{$_}\" } keys %$entry;\n \t\tforeach my $check (sort keys %$query) {\n-\t\t\tif (defined $query->{$check}) {\n+\t\t\tif (!defined $entry->{$check}) {\n+\t\t\t       log_debug(\"OK: entry has no $check token, so any value satisfies check $check\");\n+\t\t\t}\n+\t\t\telsif (defined $query->{$check}) {\n \t\t\t\tlog_debug(\"compare %s [%s] to [%s] (entry: %s)\",\n \t\t\t\t\t  $check,\n \t\t\t\t\t  $entry->{$check},\n-- \n1.8.1.5\n"},{"id":"228620","messageId":"20131008194147.GF9464@google.com","threadId":"35083","inReplyTo":"87zjqjx25g.fsf@flea.lifelogs.com","subject":"Re: [PATCH] git-credential-netrc: fix uninitialized warning","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-10-08T19:41:47Z","receivedAt":"2013-10-08T19:41:47Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nTed Zlatanov wrote:\n\n> Simple patch to avoid unitialized warning and log what we'll do.\n\nSign-off?\n\n[...]\n> --- a/contrib/credential/netrc/git-credential-netrc\n> +++ b/contrib/credential/netrc/git-credential-netrc\n> @@ -369,7 +369,10 @@ sub find_netrc_entry {\n>  \t{\n>  \t\tmy $entry_text = join ', ', map { \"$_=$entry->{$_}\" } keys %$entry;\n>  \t\tforeach my $check (sort keys %$query) {\n> -\t\t\tif (defined $query->{$check}) {\n> +\t\t\tif (!defined $entry->{$check}) {\n> +\t\t\t       log_debug(\"OK: entry has no $check token, so any value satisfies check $check\");\n> +\t\t\t}\n> +\t\t\telsif (defined $query->{$check}) {\n\nStyle: elsewhere this file seems to use cuddled elses:\n\n\t} elsif (...) {\n\nOr more simply, would it make sense to wrap both 'defined' checks into\na single \"if\", like so?\n\n\t\tif (defined $entry->{$check} && defined $query->{$check}) {\n\t\t\t...\n\t\t} else {\n\t\t\tlog_debug(...);\n\t\t}\n\nThanks and hope that helps,\nJonathan\n"},{"id":"228621","messageId":"87li23v8p5.fsf@flea.lifelogs.com","threadId":"35083","inReplyTo":"20131008194147.GF9464@google.com","subject":"Re: [PATCH] git-credential-netrc: fix uninitialized warning","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-10-08T19:55:34Z","receivedAt":"2013-10-08T19:55:34Z","isPatch":true,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"On Tue, 8 Oct 2013 12:41:47 -0700 Jonathan Nieder <jrnieder@gmail.com> wrote: \n\nJN> Ted Zlatanov wrote:\n\n>> Simple patch to avoid unitialized warning and log what we'll do.\n\nJN> Sign-off?\n\nI didn't realize it was a requirement, must I?\n\nJN> [...]\n>> --- a/contrib/credential/netrc/git-credential-netrc\n>> +++ b/contrib/credential/netrc/git-credential-netrc\n>> @@ -369,7 +369,10 @@ sub find_netrc_entry {\n>> {\n>> my $entry_text = join ', ', map { \"$_=$entry->{$_}\" } keys %$entry;\n>> foreach my $check (sort keys %$query) {\n>> -\t\t\tif (defined $query->{$check}) {\n>> +\t\t\tif (!defined $entry->{$check}) {\n>> +\t\t\t       log_debug(\"OK: entry has no $check token, so any value satisfies check $check\");\n>> +\t\t\t}\n>> +\t\t\telsif (defined $query->{$check}) {\n\nJN> Style: elsewhere this file seems to use cuddled elses:\n\nJN> \t} elsif (...) {\n\nAh, thanks, I missed that.\n\nJN> Or more simply, would it make sense to wrap both 'defined' checks into\nJN> a single \"if\", like so?\n\nJN> \t\tif (defined $entry->{$check} && defined $query->{$check}) {\nJN> \t\t\t...\nJN> \t\t} else {\nJN> \t\t\tlog_debug(...);\nJN> \t\t}\n\nI prefer the explicit version because we can issue a more precise\nlog_debug message.\n\nTed\n"},{"id":"228622","messageId":"525463F1.2050308@googlemail.com","threadId":"35083","inReplyTo":"87li23v8p5.fsf@flea.lifelogs.com","subject":"Re: [PATCH] git-credential-netrc: fix uninitialized warning","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2013-10-08T19:58:41Z","receivedAt":"2013-10-08T19:58:41Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On 10/08/2013 09:55 PM, Ted Zlatanov wrote:\n> JN> Sign-off?\n> \n> I didn't realize it was a requirement, must I?\n\nYes, this is a requirement. See Documentation/SubmittingPatches\nto read what signing off actually means here.\n\nStefan\n\n"},{"id":"228624","messageId":"20131008200235.GG9464@google.com","threadId":"35083","inReplyTo":"87li23v8p5.fsf@flea.lifelogs.com","subject":"Re: [PATCH] git-credential-netrc: fix uninitialized warning","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-10-08T20:02:35Z","receivedAt":"2013-10-08T20:02:35Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ted Zlatanov wrote:\n> On Tue, 8 Oct 2013 12:41:47 -0700 Jonathan Nieder <jrnieder@gmail.com> wrote: \n> JN> Ted Zlatanov wrote:\n\n>>> Simple patch to avoid unitialized warning and log what we'll do.\n> JN> Sign-off?\n>\n> I didn't realize it was a requirement, must I?\n\nSee Documentation/SubmittingPatches, section '(5) Sign your work'\nfor what this means.\n\nIf you just forgot to sign off, that's fine and I can forge it or go\nwithout.  If you are unable to sign off because you don't have the\nright to submit the change under an open source license, I'd be a bit\nworried going forward.\n\n[...]\n> JN> Or more simply, would it make sense to wrap both 'defined' checks into\n> JN> a single \"if\", like so?\n>\n> JN> \t\tif (defined $entry->{$check} && defined $query->{$check}) {\n> JN> \t\t\t...\n> JN> \t\t} else {\n> JN> \t\t\tlog_debug(...);\n> JN> \t\t}\n>\n> I prefer the explicit version because we can issue a more precise\n> log_debug message.\n\nThat's fine with me.\n\nAfter this patch, the code looks like\n\n\tif (!defined $entry->{$check}) {\n\t\tlog_debug(...);\n\t} elsif (defined $query->{$check}) {\n\t\t...\n\t} else {\n\t\tlog_debug(...);\n\t}\n\nAs a small nit, wouldn't it be more readable with the two !defined()\ncases together?\n\n\tif (!defined $entry->{$check}) {\n\t\t...\n\t} elsif (!defined $query->{$check}) {\n\t\t...\n\t} else {\n\t\t...\n\t}\n\nThanks again.\nJonathan\n"},{"id":"228625","messageId":"87a9ijv8ax.fsf@flea.lifelogs.com","threadId":"35083","inReplyTo":"525463F1.2050308@googlemail.com","subject":"Re: [PATCH] git-credential-netrc: fix uninitialized warning","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-10-08T20:04:06Z","receivedAt":"2013-10-08T20:04:06Z","isPatch":true,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"On Tue, 08 Oct 2013 21:58:41 +0200 Stefan Beller <stefanbeller@googlemail.com> wrote: \n\nSB> On 10/08/2013 09:55 PM, Ted Zlatanov wrote:\nJN> Sign-off?\n>> \n>> I didn't realize it was a requirement, must I?\n\nSB> Yes, this is a requirement. See Documentation/SubmittingPatches\nSB> to read what signing off actually means here.\n\nOK, I didn't realize it was.  Thanks for explaining.\n\nTed\n"},{"id":"228626","messageId":"8761t7v7w7.fsf@flea.lifelogs.com","threadId":"35083","inReplyTo":"20131008200235.GG9464@google.com","subject":"Re: [PATCH] git-credential-netrc: fix uninitialized warning","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-10-08T20:12:56Z","receivedAt":"2013-10-08T20:12:56Z","isPatch":true,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"On Tue, 8 Oct 2013 13:02:35 -0700 Jonathan Nieder <jrnieder@gmail.com> wrote: \n\nJN> Ted Zlatanov wrote:\n>> On Tue, 8 Oct 2013 12:41:47 -0700 Jonathan Nieder <jrnieder@gmail.com> wrote: \nJN> Ted Zlatanov wrote:\n\n>>>> Simple patch to avoid unitialized warning and log what we'll do.\nJN> Sign-off?\n>> \n>> I didn't realize it was a requirement, must I?\n\nJN> See Documentation/SubmittingPatches, section '(5) Sign your work'\nJN> for what this means.\n\nJN> If you just forgot to sign off, that's fine and I can forge it or go\nJN> without.  If you are unable to sign off because you don't have the\nJN> right to submit the change under an open source license, I'd be a bit\nJN> worried going forward.\n\nRight, I got it.  Sorry, I didn't know that applied to trivial patches.\n\nJN> After this patch, the code looks like\n\nJN> \tif (!defined $entry->{$check}) {\nJN> \t\tlog_debug(...);\nJN> \t} elsif (defined $query->{$check}) {\nJN> \t\t...\nJN> \t} else {\nJN> \t\tlog_debug(...);\nJN> \t}\n\nJN> As a small nit, wouldn't it be more readable with the two !defined()\nJN> cases together?\n\nJN> \tif (!defined $entry->{$check}) {\nJN> \t\t...\nJN> \t} elsif (!defined $query->{$check}) {\nJN> \t\t...\nJN> \t} else {\nJN> \t\t...\nJN> \t}\n\nBecause of way the \"next ENTRY\" line comes out, I like my way slightly\nbetter, but honestly it's fine either way :)  If you like, go ahead and\ncommit the rewrite the way it works for you, I have no objections at all.\n\nTed\n"}]}