Re: What's cooking in git.git (Apr 2012, #06; Sun, 15)
On Thu, Apr 19, 2012 at 8:31 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
Show 19 quoted lines
> On Thu, Apr 19, 2012 at 3:18 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
>> On Thu, Apr 19, 2012 at 7:58 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
>>> This approach has the problem that file-operations apart from pread
>>> might (at least in theory) modify the position. To prevent that, we'd
>>> either need to use the same locking-mechanism as the CRT use, or use
>>> ReadFile with an OVERLAPPED struct, which allows us specify the offset
>>> explicitly. The latter seems better to me, and should look something
>>> like this (note: untested):
>>
>> Yeah. I read about ReadFile [1] but dismissed it when I got to async
>> i/o mode. Reading again, sync i/o ReadFile with OVERLAPPED struct
>> should work fine. It's not clear though if file offset is changed
>> (pread man page says it does not change).
>>
>
> A quick test shows that it does not:
> ...
>
> So this looks fine to me.
Great. We now wait for good news from Johannes then.
Show 6 quoted lines
>> Also this approach deals with Windows only. There's still another
>> NO_PREAD user, HP-UX something, and NO_PREAD comment mentions cygwin
>> before v1.5.22. I personally don't care, just wanted to point out.
>
> Yeah. Other platforms are still an issue. You didn't address those
> either in your patch,
No I didn't. Your patch made me see that.
Show 6 quoted lines
> even though it would be possible to modify it to
> deal with them by checking the NO_PREAD and NO_PTHREADS defines.
>
> But they would still have the problem with the file-pointer racing for
> non-pread operations. Perhaps simply disabling threading is the better
> choice for these?
That sounds like a safe choice. I'll make a patch to do that.
--
Duy
Nguyen Thai Ngoc Duy <pclouds@gmail.com>