Re: [PATCH 1/3] Git.pm: Add faculties to allow temp files to be cached

From: Marcus Griep <marcus@griep.us>
Date: Aug 12, 2008, 15:41 UTC
Message-ID: <48A1AF28.5000400@griep.us>
In-Reply-To: <20080812030809.GA14051@untitled>

Eric Wong wrote:

> I'm not sure if the no-lock version is worth the potential for
> buggy or dangerous code.  I like this new idea of locking the
> files to prevent bugs.

I can agree with that, and the "unsafe" version was just a front to the common function. I've removed the unsafe version from @EXPORT_OK and removed temp_unsafe, but _temp_cached is still available for those that _really_ want the unsafe version.

> Always truncating on release makes the interface simpler.  With locking,
> we can probably *only* truncate on release if you're that worried about
> the extra overhead :)

I agree with this. I introduced a nice bug on myself when just starting with it though, which is why I made it optional. Careful conversion and testing should be good enough protection.

Show 7 quoted lines

> I would do a regular seek() here in addition to the sysseek() below. I
> am not certain one of the many userspace buffering layers Perl can
> potentially use doesn't do anything funky with its offset accounting.
> 
> I would also put a tell() here after the sysseek and throw an error if
> it returns a non-zero value just in case.  Yes, I'm really paranoid
> about this stuff and have a huge distrust of userspace I/O layers :)

I went ahead and threw in a sysseek(,,SEEK_CUR) with the tell and added a seek to the sysseek(,,SEEK_SET), so we should be protected on the buffered and unbuffered sides.

-- 
Marcus Griep
GPG Key ID: 0x5E968152
——
http://www.boohaunt.net
את.ψο´