gitweb: make gitweb_check_feature a boolean wrapper
The gitweb_get_feature() function retrieves the configuration parameters for the feature (such as the list of snapshot formats or the list of additional actions), but it is very often used to see if feature is enabled (which is returned as the first element in the list). Because accepting the returned list in the scalar context by mistake yields the number of elements in the array, which is non-zero in all cases, such a mistake would result in a bug for the latter use, with disabled features appearing enabled. All existing callers that call the function for this purpose assign the return value in the list context to retrieve the first element, but that is only because we fixed careless callers recently. This adds gitweb_check_feature() as a wrapper to gitweb_get_feature() that can be called safely in the scalar context to see if a feature is enabled to reduce the risk of future bugs. Callers of "get" that use the call only to see if the feature is enabled are updated to call this wrapper. Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Giuseppe Bilotta committed
Nov 29, 2008 at 13:07 UTC
25b2790fff5b7b484f1a2f40fafa4b24db1506a2
1 file changed
+36
-20
gitweb/gitweb.perl
+36
-20
@@ -190,7 +190,7 @@ our %feature = (
190
# if there is no 'sub' key (no feature-sub), then feature cannot be
191
# overriden
192
#
193
- #
193
+ # use gitweb_check_feature(<feature>) to check if <feature> is enabled
194
195
# Enable the 'blame' blob view, showing the last commit that modified
196
# each line in the file. This can be very CPU-intensive.
@@ -344,6 +344,22 @@ sub gitweb_get_feature {
344
return $sub->(@defaults);
345
}
346
347
+# A wrapper to check if a given feature is enabled.
348
+# With this, you can say
349
+#
350
+# my $bool_feat = gitweb_check_feature('bool_feat');
351
+# gitweb_check_feature('bool_feat') or somecode;
352
+#
353
+# instead of
354
+#
355
+# my ($bool_feat) = gitweb_get_feature('bool_feat');
356
+# (gitweb_get_feature('bool_feat'))[0] or somecode;
357
+#
358
+sub gitweb_check_feature {
359
+ return (gitweb_get_feature(@_))[0];
360
+}
361
+
362
+
363
sub feature_blame {
364
my ($val) = git_get_project_config('blame', '--bool');
365
@@ -810,7 +826,7 @@ sub href (%) {
826
}
827
}
828
813
- my ($use_pathinfo) = gitweb_get_feature('pathinfo');
829
+ my $use_pathinfo = gitweb_check_feature('pathinfo');
830
if ($use_pathinfo) {
831
# try to put as many parameters as possible in PATH_INFO:
832
# - project name
@@ -2101,7 +2117,7 @@ sub git_get_projects_list {
2117
$filter ||= '';
2118
$filter =~ s/\.git$//;
2119
2104
- my ($check_forks) = gitweb_get_feature('forks');
2120
+ my $check_forks = gitweb_check_feature('forks');
2121
2122
if (-d $projects_list) {
2123
# search in directory
@@ -2947,7 +2963,7 @@ EOF
2963
}
2964
print "</div>\n";
2965
2950
- my ($have_search) = gitweb_get_feature('search');
2966
+ my $have_search = gitweb_check_feature('search');
2967
if (defined $project && $have_search) {
2968
if (!defined $searchtext) {
2969
$searchtext = "";
@@ -2961,7 +2977,7 @@ EOF
2977
$search_hash = "HEAD";
2978
}
2979
my $action = $my_uri;
2964
- my ($use_pathinfo) = gitweb_get_feature('pathinfo');
2980
+ my $use_pathinfo = gitweb_check_feature('pathinfo');
2981
if ($use_pathinfo) {
2982
$action .= "/".esc_url($project);
2983
}
@@ -3454,7 +3470,7 @@ sub is_patch_split {
3470
sub git_difftree_body {
3471
my ($difftree, $hash, @parents) = @_;
3472
my ($parent) = $parents[0];
3457
- my ($have_blame) = gitweb_get_feature('blame');
3473
+ my $have_blame = gitweb_check_feature('blame');
3474
print "<div class=\"list_head\">\n";
3475
if ($#{$difftree} > 10) {
3476
print(($#{$difftree} + 1) . " files changed:\n");
@@ -3914,7 +3930,7 @@ sub fill_project_list_info {
3930
my ($projlist, $check_forks) = @_;
3931
my @projects;
3932
3917
- my ($show_ctags) = gitweb_get_feature('ctags');
3933
+ my $show_ctags = gitweb_check_feature('ctags');
3934
PROJECT:
3935
foreach my $pr (@$projlist) {
3936
my (@activity) = git_get_last_activity($pr->{'path'});
@@ -3968,7 +3984,7 @@ sub git_project_list_body {
3984
# actually uses global variable $project
3985
my ($projlist, $order, $from, $to, $extra, $no_header) = @_;
3986
3971
- my ($check_forks) = gitweb_get_feature('forks');
3987
+ my $check_forks = gitweb_check_feature('forks');
3988
my @projects = fill_project_list_info($projlist, $check_forks);
3989
3990
$order ||= $default_projects_order;
@@ -3988,7 +4004,7 @@ sub git_project_list_body {
4004
@projects = sort {$a->{$oi->{'key'}} <=> $b->{$oi->{'key'}}} @projects;
4005
}
4006
3991
- my ($show_ctags) = gitweb_get_feature('ctags');
4007
+ my $show_ctags = gitweb_check_feature('ctags');
4008
if ($show_ctags) {
4009
my %ctags;
4010
foreach my $p (@projects) {
@@ -4428,7 +4444,7 @@ sub git_summary {
4444
my @taglist = git_get_tags_list(16);
4445
my @headlist = git_get_heads_list(16);
4446
my @forklist;
4431
- my ($check_forks) = gitweb_get_feature('forks');
4447
+ my $check_forks = gitweb_check_feature('forks');
4448
4449
if ($check_forks) {
4450
@forklist = git_get_projects_list($project);
@@ -4457,7 +4473,7 @@ sub git_summary {
4473
}
4474
4475
# Tag cloud
4460
- my ($show_ctags) = gitweb_get_feature('ctags');
4476
+ my $show_ctags = gitweb_check_feature('ctags');
4477
if ($show_ctags) {
4478
my $ctags = git_get_project_ctags($project);
4479
my $cloud = git_populate_project_tagcloud($ctags);
@@ -4559,7 +4575,7 @@ sub git_blame {
4575
my $fd;
4576
my $ftype;
4577
4562
- gitweb_get_feature('blame')[0]
4578
+ gitweb_check_feature('blame')
4579
or die_error(403, "Blame view not allowed");
4580
4581
die_error(400, "No file name given") unless $file_name;
@@ -4747,7 +4763,7 @@ sub git_blob {
4763
$expires = "+1d";
4764
}
4765
4750
- my ($have_blame) = gitweb_get_feature('blame');
4766
+ my $have_blame = gitweb_check_feature('blame');
4767
open my $fd, "-|", git_cmd(), "cat-file", "blob", $hash
4768
or die_error(500, "Couldn't cat $file_name, $hash");
4769
my $mimetype = blob_mimetype($fd, $file_name);
@@ -4840,7 +4856,7 @@ sub git_tree {
4856
my $ref = format_ref_marker($refs, $hash_base);
4857
git_header_html();
4858
my $basedir = '';
4843
- my ($have_blame) = gitweb_get_feature('blame');
4859
+ my $have_blame = gitweb_check_feature('blame');
4860
if (defined $hash_base && (my %co = parse_commit($hash_base))) {
4861
my @views_nav = ();
4862
if (defined $file_name) {
@@ -5610,7 +5626,7 @@ sub git_history {
5626
}
5627
5628
sub git_search {
5613
- gitweb_get_feature('search')[0] or die_error(403, "Search is disabled");
5629
+ gitweb_check_feature('search') or die_error(403, "Search is disabled");
5630
if (!defined $searchtext) {
5631
die_error(400, "Text field is empty");
5632
}
@@ -5629,11 +5645,11 @@ sub git_search {
5645
if ($searchtype eq 'pickaxe') {
5646
# pickaxe may take all resources of your box and run for several minutes
5647
# with every query - so decide by yourself how public you make this feature
5632
- gitweb_get_feature('pickaxe')[0]
5648
+ gitweb_check_feature('pickaxe')
5649
or die_error(403, "Pickaxe is disabled");
5650
}
5651
if ($searchtype eq 'grep') {
5636
- gitweb_get_feature('grep')[0]
5652
+ gitweb_check_feature('grep')
5653
or die_error(403, "Grep is disabled");
5654
}
5655
@@ -5838,7 +5854,7 @@ insensitive).</p>
5854
<dt><b>commit</b></dt>
5855
<dd>The commit messages and authorship information will be scanned for the given pattern.</dd>
5856
EOT
5841
- my ($have_grep) = gitweb_get_feature('grep');
5857
+ my $have_grep = gitweb_check_feature('grep');
5858
if ($have_grep) {
5859
print <<EOT;
5860
<dt><b>grep</b></dt>
@@ -5855,7 +5871,7 @@ EOT
5871
<dt><b>committer</b></dt>
5872
<dd>Name and e-mail of the committer and date of commit will be scanned for the given pattern.</dd>
5873
EOT
5858
- my ($have_pickaxe) = gitweb_get_feature('pickaxe');
5874
+ my $have_pickaxe = gitweb_check_feature('pickaxe');
5875
if ($have_pickaxe) {
5876
print <<EOT;
5877
<dt><b>pickaxe</b></dt>
@@ -5907,7 +5923,7 @@ sub git_shortlog {
5923
5924
sub git_feed {
5925
my $format = shift || 'atom';
5910
- my ($have_blame) = gitweb_get_feature('blame');
5926
+ my $have_blame = gitweb_check_feature('blame');
5927
5928
# Atom: http://www.atomenabled.org/developers/syndication/
5929
# RSS: http://www.notestips.com/80256B3A007F2692/1/NAMO5P9UPQ