[PATCH] fix pread()'s short read in index-pack

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Since v1.6.0.2~13^2~ the completion of a thin pack uses sha1write() for 
its ability to compute a SHA1 on the written data.  This also provides 
data buffering which, along with commit 92392b4a45, will confuse pread() 
whenever an appended object is 1) freed due to memory pressure because 
of the depth-first delta processing, and 2) needed again because it has 
many delta children, and 3) its data is still buffered by sha1write().

Let's fix the issue by simply forcing cached data out when such an 
object is written so it can be pread()'d at leisure.

Signed-off-by: Nicolas Pitre <nico@xxxxxxx>

---

On Thu, 9 Oct 2008, Alex Riesen wrote:

> Somehow I got my gnulib mirror to a strange state where I can't fetch
> anything from it:
> 
>     $ cd gnulib
>     $ git fetch -f ../gnulib.git 'refs/heads/*:refs/remotes/origin/*'
>     remote: Counting objects: 2202, done.
>     remote: Compressing objects: 100% (633/633), done.
>     remote: Total 1769 (delta 1448), reused 1455 (delta 1134)
>     Receiving objects: 100% (1769/1769), 404.11 KiB, done.
>     fatal: premature end of pack file, 1745 bytes missing
>     fatal: index-pack failed
> 
> This is with current Shawn's master (Junio's master erroneously gives
> an error). Bisect points at ac0463ed44e859c84e354cd0ae47d9b5b124e112

Thanks for providing a good test data set.  As you can see above, this 
is something that is tricky to reproduce otherwise.  This patch is meant 
for the maint branch but should be included in master as well.

diff --git a/csum-file.c b/csum-file.c
index bb70c75..3b3e090 100644
--- a/csum-file.c
+++ b/csum-file.c
@@ -11,7 +11,7 @@
 #include "progress.h"
 #include "csum-file.h"
 
-static void sha1flush(struct sha1file *f, void *buf, unsigned int count)
+static void flush(struct sha1file *f, void *buf, unsigned int count)
 {
 	for (;;) {
 		int ret = xwrite(f->fd, buf, count);
@@ -30,22 +30,28 @@ static void sha1flush(struct sha1file *f, void *buf, unsigned int count)
 	}
 }
 
-int sha1close(struct sha1file *f, unsigned char *result, unsigned int flags)
+void sha1flush(struct sha1file *f)
 {
-	int fd;
 	unsigned offset = f->offset;
 
 	if (offset) {
 		SHA1_Update(&f->ctx, f->buffer, offset);
-		sha1flush(f, f->buffer, offset);
+		flush(f, f->buffer, offset);
 		f->offset = 0;
 	}
+}
+
+int sha1close(struct sha1file *f, unsigned char *result, unsigned int flags)
+{
+	int fd;
+
+	sha1flush(f);
 	SHA1_Final(f->buffer, &f->ctx);
 	if (result)
 		hashcpy(result, f->buffer);
 	if (flags & (CSUM_CLOSE | CSUM_FSYNC)) {
 		/* write checksum and close fd */
-		sha1flush(f, f->buffer, 20);
+		flush(f, f->buffer, 20);
 		if (flags & CSUM_FSYNC)
 			fsync_or_die(f->fd, f->name);
 		if (close(f->fd))
@@ -83,7 +89,7 @@ int sha1write(struct sha1file *f, void *buf, unsigned int count)
 		left -= nr;
 		if (!left) {
 			SHA1_Update(&f->ctx, data, offset);
-			sha1flush(f, data, offset);
+			flush(f, data, offset);
 			offset = 0;
 		}
 		f->offset = offset;
diff --git a/csum-file.h b/csum-file.h
index 72c9487..01f13b5 100644
--- a/csum-file.h
+++ b/csum-file.h
@@ -24,6 +24,7 @@ extern struct sha1file *sha1fd(int fd, const char *name);
 extern struct sha1file *sha1fd_throughput(int fd, const char *name, struct progress *tp);
 extern int sha1close(struct sha1file *, unsigned char *, unsigned int);
 extern int sha1write(struct sha1file *, void *, unsigned int);
+extern void sha1flush(struct sha1file *f);
 extern void crc32_begin(struct sha1file *);
 extern uint32_t crc32_end(struct sha1file *);
 
diff --git a/index-pack.c b/index-pack.c
index 530d820..ca72329 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -704,6 +704,7 @@ static struct object_entry *append_obj_to_pack(struct sha1file *f,
 	obj[1].idx.offset = obj[0].idx.offset + n;
 	obj[1].idx.offset += write_compressed(f, buf, size);
 	obj[0].idx.crc32 = crc32_end(f);
+	sha1flush(f);
 	hashcpy(obj->idx.sha1, sha1);
 	return obj;
 }
--
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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux