Re: [PATCH] gitweb: fixes to gitweb feature check code

On Sun, 30 Nov 2008, Giuseppe Bilotta wrote:

The gitweb_check_feature routine was being used for two different
purposes: retrieving the actual feature value (such as the list of
snapshot formats or the list of additional actions), and checking if a
feature was enabled.

This led to subtle bugs in feature checking code: gitweb_check_feature
would return (0) for disabled features, so its use in scalar context
would return true instead of false.

To fix this issue and future-proof the code, we split feature value
retrieval into its own gitweb_get_feature()function , and ensure that

  retrieval into its own gitweb_get_feature() function, and ensure that

the boolean feature check function gitweb_check_feature() always returns
a scalar (precisely, the first/only item in the feature value list).

Usage of gitweb_check_feature() across gitweb is replaced with
gitweb_get_feature() where appropriate, and list evaluation of
gitweb_check_feature() is demoted to scalar evaluation to prevent
ambiguity. The few previously incorrect uses of gitweb_check_feature()
in scalar context are left untouched because they are now correct.

Signed-off-by: Giuseppe Bilotta <redacted>

Acked-by: Jakub Narebski <redacted>


What I like about having all this, i.e. fix, futureproof and style
correction in one single patch is the fact that fix doesn't introduce
strange looking (gitweb_check_feature('bool_feat'))[0]... well, except
encapsulated in a subroutine.

From all possible splits of this feature into series of up to three

patches I think I like the one with pure subroutine rename from *check*
to *get* least...
-- 
Jakub Narebski
Poland