Even if we make that change to use TLS for this case, the
implementation of pread() will still change in such a way that the
semantics of pread() are different on Windows.  Is that ok?

Just to summarize, here's the viable approaches I've seen discussed so far:

1) Use _WINVER at compile time to select either a thread-safe or
non-thread-safe implementation of pread.  This is the easiest possible
code change, but would necessitate 2 binary distributions of git for
2) Use TLS as you suggest and have one fd per pack thread.  Probably
the most complicated code change (at least for me, being a first-time
3) Use Karsten's suggested implementation from earlier in the thread.
Seems to work, but it's a little confusing from a readability
standpoint since the implementation is not-thread safe except in this
specific usage context.

On Fri, Feb 14, 2014 at 4:56 PM, Duy Nguyen <pclo...@gmail.com> wrote:
> On Sat, Feb 15, 2014 at 7:50 AM, Stefan Zager <sza...@google.com> wrote:
>> On Fri, Feb 14, 2014 at 4:45 PM, Duy Nguyen <pclo...@gmail.com> wrote:
>>> On Sat, Feb 15, 2014 at 2:16 AM, Zachary Turner <ztur...@chromium.org> 
>>> wrote:
>>>> (Gah, sorry if you're receiving multiple emails to your personal
>>>> addresses, I need to get used to manually setting Plain-text mode
>>>> every time I send a message).
>>>> For the mixed read, we wouldn't be looking for another caller of
>>>> pread() (since it doesn't care what the file pointer is), but instead
>>>> a caller of read() or lseek() (since those do depend on the current
>>>> file pointer).  In index-pack.c, I see two possible culprits:
>>>> 1) A call to xread() from inside fill()
>>>> 2) A call to lseek in parse_pack_objects()
>>>> Do you think these could be related?  If so, maybe that opens up some
>>>> other solutions?
>>> For index-pack alone, what's wrong with open one file handle per thread?
>> Nothing wrong with that, except that it would mean either using
>> thread-local storage (which the code doesn't currently use); or
>> plumbing pack_fd through the call stack, which doesn't sound very fun.
> Current code does use thread-local storage (struct thread_local and
> get_thread_data). Adding a new file handle when NO_THREAD_SAFE_PREAD
> is defined is simpler imo.
> --
> Duy
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to