From: Johannes Sixt <j.sixt@viscovery.net>
To: "René Scharfe" <rene.scharfe@lsrfire.ath.cx>
Cc: Sebastian Schuberth <sschuberth@gmail.com>,
git@vger.kernel.org, msysgit@googlegroups.com
Subject: Re: [PATCH] Fix checkout of large files to network shares under Windows XP
Date: Tue, 20 Apr 2010 10:18:25 +0200 [thread overview]
Message-ID: <4BCD6351.1050706@viscovery.net> (raw)
In-Reply-To: <4BCCC05E.4030206@lsrfire.ath.cx>
Am 4/19/2010 22:43, schrieb René Scharfe:
> Am 19.04.2010 14:45, schrieb Sebastian Schuberth:
>> +#undef write
>> +ssize_t mingw_write(int fd, const void *buf, size_t count)
>> +{
>> + ssize_t written = 0;
>> + size_t total = 0, size = count;
>> +
>> + while (total < count && size > 0) {
>> + written = write(fd, buf, size);
>> + if (written < 0 && errno == EINVAL) {
>> + // There seems to be a bug in the Windows XP network stack that
>> + // causes writes with sizes > 64 MB to fail, so we halve the size
>> + // until we succeed or ultimately fail.
>
> C style comments (/*...*/) are preferred over C++ style comments (//...)
> for git.
>
> Is there a known-good size, or at least a mostly-working one? Would it
> make sense to start with that size instead of halving and trying until
> that size is reached?
>
>> + size /= 2;
>> + } else {
>> + buf += written;
>> + total += written;
>
> What about other errors? You need to break out of the loop instead of
> adding -1 to buf and total, right?
Thanks for a thorough review. I had the gut feeling that something's wrong
with the code due to its structure, but didn't stare at the code long
enough to notice this.
I suggest to have this structure
write
if success or failure is not EINVAL
return
do
reduce size
if larger than known (presumed?) maximum
reduce to that maximum
write
while not success and failure is EINVAL
while not failure and exactly reduced size written
write more
I don't think that we will observe any short writes *after* the size was
reduced, which Albert is concerned about. Somebody who observes the
failure that this works around could instrument the function to see
whether short writes are really a problem.
-- Hannes
next prev parent reply other threads:[~2010-04-20 8:18 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-04-19 12:45 [PATCH] Fix checkout of large files to network shares under Windows XP Sebastian Schuberth
2010-04-19 20:41 ` Junio C Hamano
2010-04-20 9:15 ` Johannes Schindelin
2010-04-19 20:43 ` René Scharfe
2010-04-19 22:46 ` Albert Dvornik
2010-04-20 8:18 ` Johannes Sixt [this message]
2010-04-20 12:42 ` Sebastian Schuberth
2010-04-20 12:57 ` Johannes Sixt
2010-04-20 14:21 ` Sebastian Schuberth
2010-04-20 20:49 ` René Scharfe
2010-04-29 20:01 ` René Scharfe
2010-04-30 8:46 ` Johannes Sixt
2010-04-30 9:08 ` Sebastian Schuberth
[not found] ` <290b11b5-5dd5-4b83-a6f5-217797ebd5af@t8g2000yqk.googlegroups.com>
2010-10-16 17:23 ` René Scharfe
2010-10-17 10:54 ` Dmitry Potapov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4BCD6351.1050706@viscovery.net \
--to=j.sixt@viscovery.net \
--cc=git@vger.kernel.org \
--cc=msysgit@googlegroups.com \
--cc=rene.scharfe@lsrfire.ath.cx \
--cc=sschuberth@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).