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

[Marcus Griep <marcus@griep.us>]

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.

> 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
את.ψο´