From: Nguyen Thai Ngoc Duy Date: Thu, 19 Apr 2012 13:38:14 GMT Subject: Re: What's cooking in git.git (Apr 2012, #06; Sun, 15) Message-ID: In-Reply-To: On Thu, Apr 19, 2012 at 8:31 PM, Erik Faye-Lund wrote: > On Thu, Apr 19, 2012 at 3:18 PM, Nguyen Thai Ngoc Duy wrote: >> On Thu, Apr 19, 2012 at 7:58 PM, Erik Faye-Lund 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. >> 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. > 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