Erik Faye-Lund <kusmabite@xxxxxxxxx> writes:
> On Tue, Jun 26, 2012 at 9:56 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxx> writes:
>>
>>> - renamed FAKE_PREAD_NOT_THREAD_SAFE to NO_THREAD_SAFE_PREAD
>>
>> Sensible.
>>
>>> - when NO_PREAD, set NO_THREAD_SAFE_PREAD in the Makefile, rather
>>> than in git-compat-util.h
>>
>> I think it is a bad change. When compat/ pread gets improved to be
>> thread-safe, this will surely be missed.
>
> But CAN it be fixed? I don't think it could, at least not without
> wrapping ALL calls to functions that perform IO on file handles or
> file descriptors...
Is that relevant? It may be true that both Erik and Junio are not
being clever enough to come up with a solution offhand. But is that
a good justification to go against a sound engineering practice?
There may be more than one platforms that want their own different
pread emulations, some safe with thread and some others not. I
think you would prefer to see something like this:
#if NO_PREAD
#if on platform A
# define NO_THREAD_SAFE_PREAD
# define pread(fd, buf, count, ofs) git_pread_A((fd), (buf), (count), (ofs))
#endif
#if on platform B
# define pread(fd, buf, count, ofs) git_pread_B((fd), (buf), (count), (ofs))
#endif
and keep "make -DNO_PREAD" to mean "The platform does not have a
pread, please emulate" and nothing else. The implementation of the
emulation itself would be the one that knows of its thread safeness.
Having said that, I do not care too deeply either way, so the patch
is queued as-is.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html