Re: [PATCHv5 03/17] gitweb/lib - Very simple file based cache
From: Jakub Narebski <hidden>
Date: 2016-06-15 22:49:43
On 7 Oct 2010, Thomas Adam wrote:
On 7 October 2010 00:00, Jakub Narebski [off-list ref] wrote:
[...]
quoted
I should really have run gitweb, caching modules and tests through perlcritic...And your other patches. :)
Well, you can't run (as far as I know) *patches* though perlcritic ;-)
quoted
quoted
quoted
+ last if $read_cnt == 0; + $size_left -= $read_cnt; + #last if $size_left <= 0; + } + + close $read_fh + or die "Couldn't close file '$file' opened for reading: $!"; + return $buf; +}"use Carp;" would be more useful here, and hence croak() and confess().For a web application we usually do not want to have too detailed error message present to client (to web browser) to avoid leaking of sensitive information.Unless we use fatalstobrowser, it will still end up in Apache's error log. I don't see a problem here.
The problem is that we do equivalent of fatalsToBrowser, i.e. set up error handler with 'set_message(\&handle_errors_html);'.
quoted
quoted
unless( defined $code and ref $code eq 'CODE' )"ref($code) eq 'CODE'" would be enough; 'undef' is not reference, and ref(undef) returns "".Sure, but it still assumes $code is a reference, and there's no guarantee of that in the code path.
Yes, but if $code is not reference, then ref($code) evaluates to empty string, and "ref($code) eq 'CODE'" comparison would fail... without any warnings. You don't need to check upfront for "defined $code"; it is the only thing I wanted to nitpick. -- Jakub Narebski Poland