Hi,
Le 26.04.2020 02:17, Jonathan Nieder a écrit :
(cc-ing Jakub Narębski, gitweb expert; Giuseppe Bilotta, who
contributed
snapshot_format)
Raphaël Gertz wrote:
Subject: commit:fix use of uninitialized value [...] in server log
This change fix the message about uninitialized value when trying to
access undefined hash indexes.
The error message fixed:
Use of uninitialized value $params{"action"} in string eq at
gitweb.cgi
line 1377
Some nitpicks about the commit message (see
Documentation/SubmittingPatches "Describe your changes well" for more
on
this subject):
- the subject line should start with the subsystem being improved, a
colon, and a space. Here, that subsystem is gitweb.
- focus on describing what improvement the patch intends to make. The
description should be in the imperative mood, as though ordering the
codebase to improve.
- try to cover what a person trying to understand whether to apply
this patch would want to know beyond what is already in the patch
itself. What user-facing behavior change does the patch make? How
was the problem discovered? What downsides are there, if any?
I think that would mean something like
gitweb: check whether query params are defined before use
In normal use, gitweb spams the web server error log:
Use of uninitialized value $params{"action"} in string eq at
gitweb.cgi line 1377
The 'action' parameter is optional so this is not warning about
anything meaningful. Check whether the parameter is defined
before using it to avoid the warning.
Signed-off-by: ...
Thank's for explaining how to improve the bug report.
Please don't. A contributor list can be obtained using "git shortlog
-n -s -- gitweb". A second changelog would fall out of sync with
that.
Didn't know sorry.
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1420,7 +1420,7 @@ sub href {
# since we destructively absorb parameters, we keep this
# boolean that remembers if we're handling a snapshot
- my $is_snapshot = $params{'action'} eq 'snapshot';
+ my $is_snapshot = defined $params{'action'} && $params{'action'} eq
'snapshot';
nit: long line
Other parameters like 'project' similarly use a defined check like
this, so it's consistent. Good.
# Summary just uses the project path URL, any other action is
# added to the URL
if (defined $params{'action'}) {
optional: should we reuse this "if"? That is, something like
my $is_snapshot = 0;
if (defined $params{'action'}) {
$is_snapshot = $params{'action'} eq 'snapshot';
$href .= ...
It may involve less risk to keep the variabe undefined:
my $is_snapshot = undef;
instead of:
my $is_snapshot = 0;
As I don't have time to maintain the changes and am not a perl Jedi,
I may have wanted to avoid rewriting lot of code.
I only intended to make minimalist code modification to avoid breaking
without noticing something and get cursed for it :)
@@ -6012,7 +6012,7 @@ sub git_history_body {
$cgi->a({-href => href(action=>$ftype, hash_base=>$commit,
file_name=>$file_name)}, $ftype) . " | " .
$cgi->a({-href => href(action=>"commitdiff", hash=>$commit)},
"commitdiff");
- if ($ftype eq 'blob') {
+ if (defined $ftype && $ftype eq 'blob') {
What is this part about? The commit message doesn't describe it.
print " | " .
$cgi->a({-href => href(action=>"blob_plain",
hash_base=>$commit, file_name=>$file_name)}, "raw");
This is an other test on possibly undefined value.
THis fix intent to remove an other warning in server log when ftype is
not defined.
Error log will look like :
gitweb.cgi: Use of uninitialized value $ftype in string eq at
/path/gitweb.cgi line 5962.: /path/gitweb.cgi
I just went to notice I missed an other error message spammed as well :
gitweb.cgi: Use of uninitialized value $commit_id in open at
/path/gitweb.cgi line 3568.: /path/gitweb.cgi
I will try to add the patch for this when possible as well.
Best regards
