Re: [PATCH 12/18] gitweb: Change file handles (in caching) to lexical variables as opposed to globs

From: J.H. <warthog9@eaglescrag.net>
Date: Dec 10, 2010, 05:56 UTC
Message-ID: <4D01C123.9050806@eaglescrag.net>
In-Reply-To: <7vwrnieac8.fsf@alter.siamese.dyndns.org>

On 12/09/2010 04:32 PM, Junio C Hamano wrote:

Show 16 quoted lines

> Jakub Narebski <jnareb@gmail.com> writes:
> 
>>> +# Global declarations
>>> +our $cacheFile;
>>> +our $cacheFileBG;
>>> +our $cacheFileBinWT;
>>> +our $cacheFileBin;
>>
>> You are trading globs for global (well, package) variables.  They are
>> not lexical filehandles... though I'm not sure if it would be possible
>> without restructuring code; note that if variable holding filehandle
>> falls out of scope, then file would be automatically closed.
> 
> Hmm. why is it a bad idea, when you need to access these from practically
> everywhere, to use global variables to begin with?  To a certain degree,
> it sounds like an unnecessary burden without much gain to me.

This was why I used globs in the first place.

To answer Jakub's question first: yes, some of those are holding locks open while things are happening. Since there's locks open, through a lot of functions, it's

Generally speaking I'm pretty good about opening and closing the files when it's needed. I *THINK* they can be made local variables, as I don't think anything is kept open (within the caching engine) across functions.

I've made a couple of changes, I'm going to have to test them, this might be fixed in v9.

- John 'Warthog9' Hawley