{"thread":{"id":"12983","subject":"[PATCH (BUGFIX)] gitweb: Fix \"Use of uninitialized value\" error in 'history' view","startedAt":"2008-04-04T14:23:42Z","lastAt":"2008-04-06T10:22:59Z","messageCount":8,"participants":["Jakub Narebski","Junio C Hamano","Gerrit Pape"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"73655","messageId":"200804041623.44595.jnareb@gmail.com","threadId":"12983","inReplyTo":null,"subject":"[PATCH (BUGFIX)] gitweb: Fix \"Use of uninitialized value\" error in 'history' view","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-04-04T14:23:42Z","receivedAt":"2008-04-04T14:23:42Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"When asked for history of a file with no $hash ('h') parameter set,\nand which file is not present in current branch (\"HEAD\" or given by\n$hash_hase ('hb') parameter), but is present deeper in the full\nhistory of a branch, gitweb would spew multiple of \"Use of\nuninitialized value\" warnings, and some links would be missing.\nThis commit fixes this bug.\n\nThis bug occurs in the rare cases when \"git log -- \" is empty\nand \"git log --full-history -- \" is not. Gitweb tried to get\nfile type (it means if it is 'tree' or 'blob' or even 'commit', as\n'history' view is for single path which can be any of given types)\nfrom the commit we start searching from, and not among found commits.\n\nWhile we are it, return error if there is _no_ history; it means that\nfile or directory was not found (for given branch). Also error out if\ntype of item could not be found: it should not happen now, but better\nbe sure.\n\nSigned-off-by: Jakub Narebski \n---\nThere should be no noticeable changes in performance for situation\nwhen bug described above does not occur; gitweb would use one git\ncommand invocation than strictly necessary in the situation which\npreviosly generated bug. As it should be rare situation (handcrafted\nURL, or \"current version\" URL for file which got deleted) I think it\nis not worth complicating the code to avoid it.\n\nBTW. the t9500-gitweb-standalone-no-errors test does not catch this\nbug, unfortunately...\n\n gitweb/gitweb.perl | 18 +++++++++++++++---\n 1 files changed, 15 insertions(+), 3 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 50922bc..1be75c6 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -5238,14 +5238,26 @@ sub git_history {\n \tmy $refs = git_get_references();\n \tmy $limit = sprintf(\"--max-count=%i\", (100 * ($page+1)));\n \n+\tmy @commitlist = parse_commits($hash_base, 101, (100 * $page),\n+\t $file_name, \"--full-history\");\n+\tif (!@commitlist) {\n+\t\tdie_error(undef, \"No such path\");\n+\t}\n+\n \tif (!defined $hash && defined $file_name) {\n-\t\t$hash = git_get_hash_by_path($hash_base, $file_name);\n+\t\t# some commits could have deleted file in question,\n+\t\t# and not have it in tree, but one of them has to have it\n+\t\tfor (my $i = 0; $i <= @commitlist; $i++) {\n+\t\t\t$hash = git_get_hash_by_path($commitlist[$i]{'id'}, $file_name);\n+\t\t\tlast if defined $hash;\n+\t\t}\n \t}\n \tif (defined $hash) {\n \t\t$ftype = git_get_type($hash);\n \t}\n-\n-\tmy @commitlist = parse_commits($hash_base, 101, (100 * $page), $file_name, \"--full-history\");\n+\tif (!defined $ftype) {\n+\t\tdie_error(undef, \"Unknown type of object\");\n+\t}\n \n \tmy $paging_nav = '';\n \tif ($page > 0) {\n-- \n1.5.4.4\n"},{"id":"73690","messageId":"7v63uwyc14.fsf@gitster.siamese.dyndns.org","threadId":"12983","inReplyTo":"200804041623.44595.jnareb@gmail.com","subject":"Re: [PATCH (BUGFIX)] gitweb: Fix \"Use of uninitialized value\" error in 'history' view","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-04-05T07:51:51Z","receivedAt":"2008-04-05T07:51:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> When asked for history of a file with no $hash ('h') parameter set,\n> and which file is not present in current branch (\"HEAD\" or given by\n> $hash_hase ('hb') parameter), but is present deeper in the full\n> history of a branch, gitweb would spew multiple of \"Use of\n> uninitialized value\" warnings, and some links would be missing.\n> This commit fixes this bug.\n\nThanks, does gitweb itself generate such a link?\n\nI can _artificially_ reproduce this, and I can also see that the patch\nwould solve it, so I do not mind applying it, but I am curious how this\nwas originally triggered.\n"},{"id":"73691","messageId":"200804051009.13332.jnareb@gmail.com","threadId":"12983","inReplyTo":"7v63uwyc14.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH (BUGFIX)] gitweb: Fix \"Use of uninitialized value\" error in 'history' view","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-04-05T08:09:11Z","receivedAt":"2008-04-05T08:09:11Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n> Jakub Narebski writes:\n> \n> > When asked for history of a file with no $hash ('h') parameter set,\n> > and which file is not present in current branch (\"HEAD\" or given by\n> > $hash_hase ('hb') parameter), but is present deeper in the full\n> > history of a branch, gitweb would spew multiple of \"Use of\n> > uninitialized value\" warnings, and some links would be missing.\n> > This commit fixes this bug.\n> \n> Thanks, does gitweb itself generate such a link?\n> \n> I can _artificially_ reproduce this, and I can also see that the patch\n> would solve it, so I do not mind applying it, but I am curious how this\n> was originally triggered.\n\nActually I came across this error when testing the \"context-sensitive\nfeed links\" patch, and was hand-crafting (munging) gitweb URLs.\n\nBut after thinking about this bug a bit I have realized that this\nsituation _can_ happen with link generated by gitweb, only in very rare\ncases. It is enough if you use 'history' view link without $hash ('h')\nparameter, and with transient $hash_base ('hb') parameter, i.e. either\nname of branch, or \"HEAD\", or not set (implies \"HEAD\"), and happen into\nsituation where file was _deleted_ *between* gitweb link creation and\nfollowing (accessing) this link. For example 'history' links in 'tree'\nand 'commitdiff' views doesn't have 'h' parameter. Also the 'history'\nlink in navigation bar wouldn't have 'h' parameter if 'blob' or 'tree'\nview where it is in doesn't have it.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"73701","messageId":"20080405164303.30566.qmail@47fa24fd520dbf.315fe32.mid.smarden.org","threadId":"12983","inReplyTo":"200804041623.44595.jnareb@gmail.com","subject":"Re: [PATCH (BUGFIX)] gitweb: Fix \"Use of uninitialized value\" error in 'history' view","fromName":"Gerrit Pape","fromEmail":"pape@smarden.org","sentAt":"2008-04-05T16:43:03Z","receivedAt":"2008-04-05T16:43:03Z","isPatch":true,"sender":{"key":"pape@smarden.org","avatar":"https://avatars.githubusercontent.com/u/143170252?v=4"},"body":"On Fri, Apr 04, 2008 at 03:23:42PM +0100, Jakub Narebski wrote:\n> When asked for history of a file with no $hash ('h') parameter set,\n> and which file is not present in current branch (\"HEAD\" or given by\n> $hash_hase ('hb') parameter), but is present deeper in the full\n> history of a branch, gitweb would spew multiple of \"Use of\n> uninitialized value\" warnings, and some links would be missing.\n> This commit fixes this bug.\n> \n> This bug occurs in the rare cases when \"git log -- \" is empty\n> and \"git log --full-history -- \" is not. Gitweb tried to get\n> file type (it means if it is 'tree' or 'blob' or even 'commit', as\n> 'history' view is for single path which can be any of given types)\n> from the commit we start searching from, and not among found commits.\n\nDo you know whether this fixes http://bugs.debian.org/469083 too? I\ntook a short look back then to fix it, but didn't manage it in a\nreasonable amount of time.\n\nThanks, Gerrit.\n"},{"id":"73702","messageId":"200804051916.14568.jnareb@gmail.com","threadId":"12983","inReplyTo":"20080405164303.30566.qmail@47fa24fd520dbf.315fe32.mid.smarden.org","subject":"Re: [PATCH (BUGFIX)] gitweb: Fix \"Use of uninitialized value\" error in 'history' view","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-04-05T17:16:11Z","receivedAt":"2008-04-05T17:16:11Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sat, 5 Apr 2008, Gerrit Pape wrote:\n> On Fri, Apr 04, 2008 at 03:23:42PM +0100, Jakub Narebski wrote:\n\n> > When asked for history of a file with no $hash ('h') parameter set,\n> > and which file is not present in current branch (\"HEAD\" or given by\n> > $hash_hase ('hb') parameter), but is present deeper in the full\n> > history of a branch, gitweb would spew multiple of \"Use of\n> > uninitialized value\" warnings, and some links would be missing.\n> > This commit fixes this bug.\n> > \n> > This bug occurs in the rare cases when \"git log -- \" is empty\n> > and \"git log --full-history -- \" is not. Gitweb tried to get\n> > file type (it means if it is 'tree' or 'blob' or even 'commit', as\n> > 'history' view is for single path which can be any of given types)\n> > from the commit we start searching from, and not among found commits.\n> \n> Do you know whether this fixes http://bugs.debian.org/469083 too? I\n> took a short look back then to fix it, but didn't manage it in a\n> reasonable amount of time.\n\nNo it does not.\n\n\nI have found what causes this bug. When adding is_deleted() subroutine\nI have forgot about old/legacy URL support[1], where gitweb doesn't have\nenough information to get raw diff info, and gitweb has to generate\nrequired info itself, i.e. the part in git_blobdiff(), around\n\n\t# old/legacy style URI\n\tif (!%diffinfo && # if new style URI failed\n\t defined $hash && defined $hash_parent) {\n\t\t# fake git-diff-tree raw output\n\t\t$diffinfo{'from_mode'} = $diffinfo{'to_mode'} = \"blob\";\n\t\t$diffinfo{'from_id'} = $hash_parent;\n\t\t$diffinfo{'to_id'} = $hash;\n\nThis code does not, and cannot, fill 'status_str' ('status' is faked).\nI'd send in a bit patch fixing this bug (making is_deleted() more\nrobust).\n\nFootnotes:\n==========\n[1] Note that example URL in http://bugs.debian.org/469083 does not\n have 'hpb' ($hash_parent_base) parameter set... and that is what\n causes this bug.\n\nP.S. Gerrit, are you maintainer of git-core debian package?\n-- \nJakub Narebski\nPoland\n"},{"id":"73704","messageId":"200804051938.17855.jnareb@gmail.com","threadId":"12983","inReplyTo":"200804051916.14568.jnareb@gmail.com","subject":"Re: [PATCH (BUGFIX)] gitweb: Fix \"Use of uninitialized value\" error in 'history' view","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-04-05T17:38:17Z","receivedAt":"2008-04-05T17:38:17Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Jakub Narebski wrote:\n> On Sat, 5 Apr 2008, Gerrit Pape wrote:\n>> On Fri, Apr 04, 2008 at 03:23:42PM +0100, Jakub Narebski wrote:\n>>\n>> Do you know whether this fixes http://bugs.debian.org/469083 too? I\n>> took a short look back then to fix it, but didn't manage it in a\n>> reasonable amount of time.\n> \n> No it does not.\n> \n> \n> I have found what causes this bug. [...]\n> I'd send in a bit patch fixing this bug (making is_deleted() more\n> robust).\n\nIt should be enough to revert commit 6aa6f92 (gitweb: Add 'status_str'\nto parse_difftree_raw_line output); it should fix this bug.\n\nP.S. Mentioned commit was done for micro-optimization: \" =~ /D/\"\nis a bit faster than \" eq ('0' x 40)\" in the case when it doesn't\nmatch.\n-- \nJakub Narebski\nPoland\n"},{"id":"73712","messageId":"200804052213.31253.jnareb@gmail.com","threadId":"12983","inReplyTo":"200804051938.17855.jnareb@gmail.com","subject":"[PATCH] Revert \"gitweb: Add 'status_str' to parse_difftree_raw_line output\"","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-04-05T20:13:24Z","receivedAt":"2008-04-05T20:13:24Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"This reverts commit 6aa6f92fda47cc4ee5f599895e8a5a327fb6f9ab.\n\nIt caused is_deleted() subroutine to output warnings when dealing with\nold, legacy gitweb blobdiff URLs without either 'hb' or 'hpb'\nparameters.\n\nThis fixes http://bugs.debian.org/469083\n\nSigned-off-by: Jakub Narebski \n---\n> It should be enough to revert commit 6aa6f92 (gitweb: Add 'status_str'\n> to parse_difftree_raw_line output); it should fix this bug.\n\nAnd here it is...\n\n gitweb/gitweb.perl | 5 ++---\n 1 files changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 1be75c6..1501ec8 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -2204,7 +2204,7 @@ sub parse_difftree_raw_line {\n \t\t$res{'to_mode'} = $2;\n \t\t$res{'from_id'} = $3;\n \t\t$res{'to_id'} = $4;\n-\t\t$res{'status'} = $res{'status_str'} = $5;\n+\t\t$res{'status'} = $5;\n \t\t$res{'similarity'} = $6;\n \t\tif ($res{'status'} eq 'R' || $res{'status'} eq 'C') { # renamed or copied\n \t\t\t($res{'from_file'}, $res{'to_file'}) = map { unquote($_) } split(\"\\t\", $7);\n@@ -2220,7 +2220,6 @@ sub parse_difftree_raw_line {\n \t\t$res{'to_mode'} = pop @{$res{'from_mode'}};\n \t\t$res{'from_id'} = [ split(' ', $3) ];\n \t\t$res{'to_id'} = pop @{$res{'from_id'}};\n-\t\t$res{'status_str'} = $4;\n \t\t$res{'status'} = [ split('', $4) ];\n \t\t$res{'to_file'} = unquote($5);\n \t}\n@@ -3068,7 +3067,7 @@ sub fill_from_file_info {\n sub is_deleted {\n \tmy $diffinfo = shift;\n \n-\treturn $diffinfo->{'status_str'} =~ /D/;\n+\treturn $diffinfo->{'to_id'} eq ('0' x 40);\n }\n \n # does patch correspond to [previous] difftree raw line\n-- \n1.5.4.4\n"},{"id":"73737","messageId":"20080406102300.1915.qmail@0406979e53f500.315fe32.mid.smarden.org","threadId":"12983","inReplyTo":"200804051916.14568.jnareb@gmail.com","subject":"Re: [PATCH (BUGFIX)] gitweb: Fix \"Use of uninitialized value\" error in 'history' view","fromName":"Gerrit Pape","fromEmail":"pape@smarden.org","sentAt":"2008-04-06T10:22:59Z","receivedAt":"2008-04-06T10:22:59Z","isPatch":true,"sender":{"key":"pape@smarden.org","avatar":"https://avatars.githubusercontent.com/u/143170252?v=4"},"body":"On Sat, Apr 05, 2008 at 06:16:11PM +0100, Jakub Narebski wrote:\n> This code does not, and cannot, fill 'status_str' ('status' is faked).\n> I'd send in a bit patch fixing this bug (making is_deleted() more\n> robust).\n> \n> Footnotes:\n> ==========\n> [1] Note that example URL in http://bugs.debian.org/469083 does not\n> have 'hpb' ($hash_parent_base) parameter set... and that is what\n> causes this bug.\n\nThanks a lot.\n\n> P.S. Gerrit, are you maintainer of git-core debian package?\n\nYes, I take responsibility for the git Debian packages.\n\nRegards, Gerrit.\n"}]}