# 'git fast-export' is crashing on the gcc repo 10 messages from 2007-12-11 to 2007-12-12. Participants: Nicolas Pitre, Marco Costalba, Alex Riesen, Junio C Hamano, David Kastrup. Thread: https://gitlist.dev/t/11244 ## Nicolas Pitre, 2007-12-11 20:27 Subject: 'git fast-export' is crashing on the gcc repo Message-ID: URL: https://gitlist.dev/e/alpine.LFD.0.99999.0712111509270.555%40xanadu.home ``` Simply doing something like: $ git fast-export --all > /dev/null results in: fatal: Could not write blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 Since this is extremely enlightening, I patched it as follows: diff --git a/builtin-fast-export.c b/builtin-fast-export.c index 2136aad..5c7bfe0 100755 --- a/builtin-fast-export.c +++ b/builtin-fast-export.c @@ -104,7 +104,8 @@ static void handle_object(const unsigned char *sha1) printf("blob\nmark :%d\ndata %lu\n", last_idnum, size); if (fwrite(buf, size, 1, stdout) != 1) - die ("Could not write blob %s", sha1_to_hex(sha1)); + die ("Could not write blob %s: %s", + sha1_to_hex(sha1), strerror(errno)); printf("\n"); show_progress(); And then running it again produced: fatal: Could not write blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391: Inappropriate ioctl for device adding to today's confusion. Nicolas ``` ## Marco Costalba, 2007-12-11 20:35 Subject: Re: 'git fast-export' is crashing on the gcc repo Message-ID: URL: https://gitlist.dev/e/e5bfff550712111235q10177d9bif314580127a56d67%40mail.gmail.com In-Reply-To: ``` On Dec 11, 2007 9:27 PM, Nicolas Pitre wrote: > > And then running it again produced: > > fatal: Could not write blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391: Inappropriate ioctl for device > > adding to today's confusion. > GCC repo is turning out to be a very good test case indeed ;-) ``` ## Alex Riesen, 2007-12-11 22:01 Subject: [PATCH] Fix git-fast-export for zero-sized blobs Message-ID: <20071211220128.GA19857@steel.home> URL: https://gitlist.dev/e/20071211220128.GA19857%40steel.home In-Reply-To: ``` Signed-off-by: Alex Riesen --- builtin-fast-export.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) Nicolas Pitre, Tue, Dec 11, 2007 21:27:33 +0100: > Simply doing something like: > > $ git fast-export --all > /dev/null > > results in: > > fatal: Could not write blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > I get this for my git repo: $ git fast-export --all >/dev/null fatal: Could not write blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 $ git cat-file blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 |wc -c 0 Writing a zero-size blob will surely returns 0. diff --git a/builtin-fast-export.c b/builtin-fast-export.c index 2136aad..ef27eee 100755 --- a/builtin-fast-export.c +++ b/builtin-fast-export.c @@ -103,7 +103,7 @@ static void handle_object(const unsigned char *sha1) mark_object(object); printf("blob\nmark :%d\ndata %lu\n", last_idnum, size); - if (fwrite(buf, size, 1, stdout) != 1) + if (size && fwrite(buf, size, 1, stdout) != 1) die ("Could not write blob %s", sha1_to_hex(sha1)); printf("\n"); -- 1.5.3.7.1177.gb22b ``` ## Nicolas Pitre, 2007-12-11 22:06 Subject: Re: 'git fast-export' is crashing on the gcc repo Message-ID: URL: https://gitlist.dev/e/alpine.LFD.0.99999.0712111703380.555%40xanadu.home In-Reply-To: ``` On Tue, 11 Dec 2007, Nicolas Pitre wrote: > Simply doing something like: > > $ git fast-export --all > /dev/null > > results in: > > fatal: Could not write blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > > Since this is extremely enlightening, I patched it as follows: > > diff --git a/builtin-fast-export.c b/builtin-fast-export.c > index 2136aad..5c7bfe0 100755 > --- a/builtin-fast-export.c > +++ b/builtin-fast-export.c > @@ -104,7 +104,8 @@ static void handle_object(const unsigned char *sha1) > > printf("blob\nmark :%d\ndata %lu\n", last_idnum, size); > if (fwrite(buf, size, 1, stdout) != 1) > - die ("Could not write blob %s", sha1_to_hex(sha1)); > + die ("Could not write blob %s: %s", > + sha1_to_hex(sha1), strerror(errno)); > printf("\n"); > > show_progress(); > > And then running it again produced: > > fatal: Could not write blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391: Inappropriate ioctl for device > > adding to today's confusion. Well, ignore the above. It seems that most of stdio doesn't set errno so the above is crap. Even strace doesn't show any error. But, somehow, the following patch fixes it: diff --git a/builtin-fast-export.c b/builtin-fast-export.c index 2136aad..c32a124 100755 --- a/builtin-fast-export.c +++ b/builtin-fast-export.c @@ -103,7 +103,7 @@ static void handle_object(const unsigned char *sha1) mark_object(object); printf("blob\nmark :%d\ndata %lu\n", last_idnum, size); - if (fwrite(buf, size, 1, stdout) != 1) + if (fwrite(buf, 1, size, stdout) != size) die ("Could not write blob %s", sha1_to_hex(sha1)); printf("\n"); Go figure. Nicolas ``` ## Nicolas Pitre, 2007-12-11 22:20 Subject: Re: [PATCH] Fix git-fast-export for zero-sized blobs Message-ID: URL: https://gitlist.dev/e/alpine.LFD.0.99999.0712111719380.555%40xanadu.home In-Reply-To: <20071211220128.GA19857@steel.home> ``` On Tue, 11 Dec 2007, Alex Riesen wrote: > Signed-off-by: Alex Riesen > --- > builtin-fast-export.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > Nicolas Pitre, Tue, Dec 11, 2007 21:27:33 +0100: > > Simply doing something like: > > > > $ git fast-export --all > /dev/null > > > > results in: > > > > fatal: Could not write blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > > > > I get this for my git repo: > > $ git fast-export --all >/dev/null > fatal: Could not write blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > $ git cat-file blob e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 |wc -c > 0 > > Writing a zero-size blob will surely returns 0. Indeed. /me looking at too many piece of code at the same time Nicolas ``` ## Alex Riesen, 2007-12-12 01:38 Subject: Re: 'git fast-export' is crashing on the gcc repo Message-ID: <20071212013828.GC19857@steel.home> URL: https://gitlist.dev/e/20071212013828.GC19857%40steel.home In-Reply-To: ``` Nicolas Pitre, Tue, Dec 11, 2007 23:06:42 +0100: > > Well, ignore the above. It seems that most of stdio doesn't set errno > so the above is crap. > Well, it had no reason to in this case. It's not an error. It does not even have to do a syscall. > diff --git a/builtin-fast-export.c b/builtin-fast-export.c > index 2136aad..c32a124 100755 > --- a/builtin-fast-export.c > +++ b/builtin-fast-export.c > @@ -103,7 +103,7 @@ static void handle_object(const unsigned char *sha1) > mark_object(object); > > printf("blob\nmark :%d\ndata %lu\n", last_idnum, size); > - if (fwrite(buf, size, 1, stdout) != 1) > + if (fwrite(buf, 1, size, stdout) != size) That's a probable syscall which could be spared ``` ## Nicolas Pitre, 2007-12-12 01:45 Subject: Re: 'git fast-export' is crashing on the gcc repo Message-ID: URL: https://gitlist.dev/e/alpine.LFD.0.99999.0712112045040.555%40xanadu.home In-Reply-To: <20071212013828.GC19857@steel.home> ``` On Wed, 12 Dec 2007, Alex Riesen wrote: > Nicolas Pitre, Tue, Dec 11, 2007 23:06:42 +0100: > > > > Well, ignore the above. It seems that most of stdio doesn't set errno > > so the above is crap. > > > > Well, it had no reason to in this case. It's not an error. > It does not even have to do a syscall. Which is why I later agreed with your patch. Nicolas ``` ## Junio C Hamano, 2007-12-12 08:32 Subject: Re: 'git fast-export' is crashing on the gcc repo Message-ID: <7v3au89utj.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7v3au89utj.fsf%40gitster.siamese.dyndns.org In-Reply-To: ``` Nicolas Pitre writes: > On Wed, 12 Dec 2007, Alex Riesen wrote: > >> Nicolas Pitre, Tue, Dec 11, 2007 23:06:42 +0100: >> > >> > Well, ignore the above. It seems that most of stdio doesn't set errno >> > so the above is crap. >> > >> >> Well, it had no reason to in this case. It's not an error. >> It does not even have to do a syscall. > > Which is why I later agreed with your patch. Still, I like your swapping of size and nmemb parameters, regardless of the "don't bother calling fwrite(3) if size is zero" fix. We are writing 1 element of size n-byte and expecting the call to return 1. It may be argued that writing one element of size 0-byte should always write 1 element (without actually having to go down to write(2), obviously) successfully and returning 0 from fwrite(3) is a bug ;-) No, I am just kidding. I checked with POSIX and it clearly says it should return 0 if size or nmemb is zero. ``` ## David Kastrup, 2007-12-12 08:46 Subject: Re: 'git fast-export' is crashing on the gcc repo Message-ID: <854peo8fm5.fsf@lola.goethe.zz> URL: https://gitlist.dev/e/854peo8fm5.fsf%40lola.goethe.zz In-Reply-To: <7v3au89utj.fsf@gitster.siamese.dyndns.org> ``` Junio C Hamano writes: > Nicolas Pitre writes: > >> On Wed, 12 Dec 2007, Alex Riesen wrote: >> >>> Nicolas Pitre, Tue, Dec 11, 2007 23:06:42 +0100: >>> > >>> > Well, ignore the above. It seems that most of stdio doesn't set errno >>> > so the above is crap. >>> > >>> >>> Well, it had no reason to in this case. It's not an error. >>> It does not even have to do a syscall. >> >> Which is why I later agreed with your patch. > > Still, I like your swapping of size and nmemb parameters, regardless > of the "don't bother calling fwrite(3) if size is zero" fix. I don't. Far too obscure, looks like an unintentional wart waiting to be corrected. I think that the explicit test is the way to go here. -- David Kastrup, Kriemhildstr. 15, 44793 Bochum ``` ## Junio C Hamano, 2007-12-12 08:56 Subject: Re: 'git fast-export' is crashing on the gcc repo Message-ID: <7vy7c08f5o.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vy7c08f5o.fsf%40gitster.siamese.dyndns.org In-Reply-To: <854peo8fm5.fsf@lola.goethe.zz> ``` David Kastrup writes: > Junio C Hamano writes: > ... >> Still, I like your swapping of size and nmemb parameters, regardless >> of the "don't bother calling fwrite(3) if size is zero" fix. > > I don't. Far too obscure, looks like an unintentional wart waiting to > be corrected. Oh, I did not mean it in the sense that would be a bugfix, but in the sense that we are writing N instances of 1 byte, not 1 instance of N byte blob, and should express size and nmemb parameters to fwrite(3) as such. IOW, I would have preferred: if (size && fwrite(buf, 1, size, stdout) != size) barf(...); ```