{"thread":{"id":"49708","subject":"[PATCH] Prevent warning","startedAt":"2018-10-29T15:39:33Z","lastAt":"2018-10-29T16:33:15Z","messageCount":3,"participants":["Pete","Torsten Bögershausen","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"361910","messageId":"01020166c07adf27-0c2cb5ac-2292-4ce3-808e-b627202fd45f-000000@eu-west-1.amazonses.com","threadId":"49708","inReplyTo":null,"subject":"[PATCH] Prevent warning","fromName":"Pete","fromEmail":"pietro.cagnoni@gmail.com","sentAt":"2018-10-29T15:39:30Z","receivedAt":"2018-10-29T15:39:33Z","isPatch":true,"sender":{"key":"pietro.cagnoni@gmail.com","avatar":null},"body":"Prevent the following warning in the web server error log:\ngitweb.cgi: Odd number of elements in anonymous hash at /usr/share/gitweb/gitweb.cgi line 3305\n---\n gitweb/gitweb.perl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 2594a4badb3d7..200647b683225 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3302,7 +3302,7 @@ sub git_get_remotes_list {\n \t\tnext if $wanted and not $remote eq $wanted;\n \t\tmy ($url, $key) = ($1, $2);\n \n-\t\t$remotes{$remote} ||= { 'heads' => () };\n+\t\t$remotes{$remote} ||= { 'heads' => [] };\n \t\t$remotes{$remote}{$key} = $url;\n \t}\n \tclose $fd or return;\n\n--\nhttps://github.com/git/git/pull/548\n"},{"id":"361913","messageId":"20181029155453.GA9819@tor.lan","threadId":"49708","inReplyTo":"01020166c07adf27-0c2cb5ac-2292-4ce3-808e-b627202fd45f-000000@eu-west-1.amazonses.com","subject":"Re: [PATCH] Prevent warning","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2018-10-29T15:54:53Z","receivedAt":"2018-10-29T15:54:58Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"Thanks for the patch.\nCould you please sign it off ?\nThe other remark would be if the header line could be written longer than\njust\n\"Prevent warning\".\nto give people digging into the Git history an initial information,\nwhere the warning occured and from which module it was caused.\nMay be something like this as a head line ?\n\ngitweb.perl: Fix Odd number of elements warning\n\n\n\nOn Mon, Oct 29, 2018 at 03:39:30PM +0000, Pete wrote:\n> Prevent the following warning in the web server error log:\n> gitweb.cgi: Odd number of elements in anonymous hash at /usr/share/gitweb/gitweb.cgi line 3305\n> ---\n>  gitweb/gitweb.perl | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 2594a4badb3d7..200647b683225 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -3302,7 +3302,7 @@ sub git_get_remotes_list {\n>  \t\tnext if $wanted and not $remote eq $wanted;\n>  \t\tmy ($url, $key) = ($1, $2);\n>  \n> -\t\t$remotes{$remote} ||= { 'heads' => () };\n> +\t\t$remotes{$remote} ||= { 'heads' => [] };\n>  \t\t$remotes{$remote}{$key} = $url;\n>  \t}\n>  \tclose $fd or return;\n> \n> --\n> https://github.com/git/git/pull/548\n"},{"id":"361919","messageId":"20181029163312.GB31497@sigill.intra.peff.net","threadId":"49708","inReplyTo":"01020166c07adf27-0c2cb5ac-2292-4ce3-808e-b627202fd45f-000000@eu-west-1.amazonses.com","subject":"Re: [PATCH] Prevent warning","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-29T16:33:12Z","receivedAt":"2018-10-29T16:33:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 29, 2018 at 03:39:30PM +0000, Pete wrote:\n\n> Prevent the following warning in the web server error log:\n> gitweb.cgi: Odd number of elements in anonymous hash at /usr/share/gitweb/gitweb.cgi line 3305\n> [...]\n> -\t\t$remotes{$remote} ||= { 'heads' => () };\n> +\t\t$remotes{$remote} ||= { 'heads' => [] };\n\nThis will indeed silence the warning, but the end result is different.\nIn the original, 'heads' would be set to undef, but now it is set to an\nempty array reference.\n\nDo the later users of the struct care about the distinction?\n\nGrepping around, I see two callers that look at \"heads\":\n\n 1. fill_remote_heads() assigns to it unconditionally, overwriting\n    whatever was there before.\n\n 2. git_remote_block() dereferences it as an array reference, and so\n    probably would have complained if we ever got there with the undef.\n    But we never would, because every call to git_remote_block() is\n    preceded by a call to fill_remote_heads().\n\nSo nobody actually ever looks at the value we set here. Is an empty\narray reference better than undef as a default value? I'd actually argue\nno. If we add new code that does not call fill_remote_heads(), it's\nprobably better for it to be undef to distinguish it from the empty list\n(and possibly raise an error that would tell us that we forgot to call\nfill_remote_heads().\n\nSo I'd say that:\n\n  $remotes{$remote} ||= { heads => undef };\n\nis a preferable conversion. Or simply deleting the line entirely, which\nhas roughly the same effect.\n\n-Peff\n"}]}