delta: refactor git_delta functions for consistency

Refactor the git_delta functions to have consistent naming and
parameters with the rest of the library.

src/delta.c

@@ -114,7 +114,7 @@ struct index_entry {
 struct git_delta_index {
 	unsigned long memsize;
 	const void *src_buf;
-	unsigned long src_size;
+	size_t src_size;
 	unsigned int hash_mask;
 	struct index_entry *hash[GIT_FLEX_ARRAY];
 };
@@ -142,8 +142,8 @@ static int lookup_index_alloc(
 	return 0;
 }
 
-struct git_delta_index *
+int git_delta_index_init(
-git_delta_create_index(const void *buf, unsigned long bufsize)
+	git_delta_index **out, const void *buf, size_t bufsize)
 {
 	unsigned int i, hsize, hmask, entries, prev_val, *hash_count;
 	const unsigned char *data, *buffer = buf;
@@ -152,8 +152,10 @@ git_delta_create_index(const void *buf, unsigned long bufsize)
 	void *mem;
 	unsigned long memsize;
 
+	*out = NULL;
+
 	if (!buf || !bufsize)
-		return NULL;
+		return 0;
 
 	/* Determine index hash size.  Note that indexing skips the
 	   first byte to allow for optimizing the rabin polynomial
@@ -172,7 +174,7 @@ git_delta_create_index(const void *buf, unsigned long bufsize)
 	hmask = hsize - 1;
 
 	if (lookup_index_alloc(&mem, &memsize, entries, hsize) < 0)
-		return NULL;
+		return -1;
 
 	index = mem;
 	mem = index->hash;
@@ -190,7 +192,7 @@ git_delta_create_index(const void *buf, unsigned long bufsize)
 	hash_count = git__calloc(hsize, sizeof(*hash_count));
 	if (!hash_count) {
 		git__free(index);
-		return NULL;
+		return -1;
 	}
 
 	/* then populate the index */
@@ -243,20 +245,20 @@ git_delta_create_index(const void *buf, unsigned long bufsize)
 	}
 	git__free(hash_count);
 
-	return index;
+	*out = index;
+	return 0;
 }
 
-void git_delta_free_index(struct git_delta_index *index)
+void git_delta_index_free(git_delta_index *index)
 {
 	git__free(index);
 }
 
-unsigned long git_delta_sizeof_index(struct git_delta_index *index)
+size_t git_delta_index_size(git_delta_index *index)
 {
-	if (index)
+	assert(index);
+
 	return index->memsize;
-	else
-		return 0;
 }
 
 /*
@@ -265,55 +267,57 @@ unsigned long git_delta_sizeof_index(struct git_delta_index *index)
  */
 #define MAX_OP_SIZE	(5 + 5 + 1 + RABIN_WINDOW + 7)
 
-void *
+int git_delta_create_from_index(
-git_delta_create(
+	void **out,
+	size_t *out_len,
 	const struct git_delta_index *index,
 	const void *trg_buf,
-	unsigned long trg_size,
+	size_t trg_size,
-	unsigned long *delta_size,
+	size_t max_size)
-	unsigned long max_size)
 {
-	unsigned int i, outpos, outsize, moff, msize, val;
+	unsigned int i, bufpos, bufsize, moff, msize, val;
 	int inscnt;
 	const unsigned char *ref_data, *ref_top, *data, *top;
-	unsigned char *out;
+	unsigned char *buf;
+
+	*out = NULL;
+	*out_len = 0;
 
 	if (!trg_buf || !trg_size)
-		return NULL;
+		return 0;
 
-	outpos = 0;
+	bufpos = 0;
-	outsize = 8192;
+	bufsize = 8192;
-	if (max_size && outsize >= max_size)
+	if (max_size && bufsize >= max_size)
-		outsize = (unsigned int)(max_size + MAX_OP_SIZE + 1);
+		bufsize = (unsigned int)(max_size + MAX_OP_SIZE + 1);
-	out = git__malloc(outsize);
+	buf = git__malloc(bufsize);
-	if (!out)
+	GITERR_CHECK_ALLOC(buf);
-		return NULL;
 
 	/* store reference buffer size */
 	i = index->src_size;
 	while (i >= 0x80) {
-		out[outpos++] = i | 0x80;
+		buf[bufpos++] = i | 0x80;
 		i >>= 7;
 	}
-	out[outpos++] = i;
+	buf[bufpos++] = i;
 
 	/* store target buffer size */
 	i = trg_size;
 	while (i >= 0x80) {
-		out[outpos++] = i | 0x80;
+		buf[bufpos++] = i | 0x80;
 		i >>= 7;
 	}
-	out[outpos++] = i;
+	buf[bufpos++] = i;
 
 	ref_data = index->src_buf;
 	ref_top = ref_data + index->src_size;
 	data = trg_buf;
 	top = (const unsigned char *) trg_buf + trg_size;
 
-	outpos++;
+	bufpos++;
 	val = 0;
 	for (i = 0; i < RABIN_WINDOW && data < top; i++, data++) {
-		out[outpos++] = *data;
+		buf[bufpos++] = *data;
 		val = ((val << 8) | *data) ^ T[val >> RABIN_SHIFT];
 	}
 	inscnt = i;
@@ -350,11 +354,11 @@ git_delta_create(
 
 		if (msize < 4) {
 			if (!inscnt)
-				outpos++;
+				bufpos++;
-			out[outpos++] = *data++;
+			buf[bufpos++] = *data++;
 			inscnt++;
 			if (inscnt == 0x7f) {
-				out[outpos - inscnt - 1] = inscnt;
+				buf[bufpos - inscnt - 1] = inscnt;
 				inscnt = 0;
 			}
 			msize = 0;
@@ -368,14 +372,14 @@ git_delta_create(
 					msize++;
 					moff--;
 					data--;
-					outpos--;
+					bufpos--;
 					if (--inscnt)
 						continue;
-					outpos--;  /* remove count slot */
+					bufpos--;  /* remove count slot */
 					inscnt--;  /* make it -1 */
 					break;
 				}
-				out[outpos - inscnt - 1] = inscnt;
+				buf[bufpos - inscnt - 1] = inscnt;
 				inscnt = 0;
 			}
 
@@ -383,22 +387,22 @@ git_delta_create(
 			left = (msize < 0x10000) ? 0 : (msize - 0x10000);
 			msize -= left;
 
-			op = out + outpos++;
+			op = buf + bufpos++;
 			i = 0x80;
 
 			if (moff & 0x000000ff)
-				out[outpos++] = moff >> 0,  i |= 0x01;
+				buf[bufpos++] = moff >> 0,  i |= 0x01;
 			if (moff & 0x0000ff00)
-				out[outpos++] = moff >> 8,  i |= 0x02;
+				buf[bufpos++] = moff >> 8,  i |= 0x02;
 			if (moff & 0x00ff0000)
-				out[outpos++] = moff >> 16, i |= 0x04;
+				buf[bufpos++] = moff >> 16, i |= 0x04;
 			if (moff & 0xff000000)
-				out[outpos++] = moff >> 24, i |= 0x08;
+				buf[bufpos++] = moff >> 24, i |= 0x08;
 
 			if (msize & 0x00ff)
-				out[outpos++] = msize >> 0, i |= 0x10;
+				buf[bufpos++] = msize >> 0, i |= 0x10;
 			if (msize & 0xff00)
-				out[outpos++] = msize >> 8, i |= 0x20;
+				buf[bufpos++] = msize >> 8, i |= 0x20;
 
 			*op = i;
 
@@ -415,31 +419,33 @@ git_delta_create(
 			}
 		}
 
-		if (outpos >= outsize - MAX_OP_SIZE) {
+		if (bufpos >= bufsize - MAX_OP_SIZE) {
-			void *tmp = out;
+			void *tmp = buf;
-			outsize = outsize * 3 / 2;
+			bufsize = bufsize * 3 / 2;
-			if (max_size && outsize >= max_size)
+			if (max_size && bufsize >= max_size)
-				outsize = max_size + MAX_OP_SIZE + 1;
+				bufsize = max_size + MAX_OP_SIZE + 1;
-			if (max_size && outpos > max_size)
+			if (max_size && bufpos > max_size)
 				break;
-			out = git__realloc(out, outsize);
+			buf = git__realloc(buf, bufsize);
-			if (!out) {
+			if (!buf) {
 				git__free(tmp);
-				return NULL;
+				return -1;
 			}
 		}
 	}
 
 	if (inscnt)
-		out[outpos - inscnt - 1] = inscnt;
+		buf[bufpos - inscnt - 1] = inscnt;
 
-	if (max_size && outpos > max_size) {
+	if (max_size && bufpos > max_size) {
-		git__free(out);
+		giterr_set(GITERR_NOMEMORY, "delta would be larger than maximum size");
-		return NULL;
+		git__free(buf);
+		return GIT_EBUFS;
 	}
 
-	*delta_size = outpos;
+	*out_len = bufpos;
-	return out;
+	*out = buf;
+	return 0;
 }
 
 /*
@@ -459,8 +465,11 @@ static int hdr_sz(
 	unsigned int c, shift = 0;
 
 	do {
-		if (d == end)
+		if (d == end) {
+			giterr_set(GITERR_INVALID, "truncated delta");
 			return -1;
+		}
+
 		c = *d++;
 		r |= (c & 0x7f) << shift;
 		shift += 7;

src/delta.h

@@ -8,11 +8,10 @@
 #include "common.h"
 #include "pack.h"
 
-/* opaque object for delta index */
+typedef struct git_delta_index git_delta_index;
-struct git_delta_index;
 
 /*
- * create_delta_index: compute index data from given buffer
+ * git_delta_index_init: compute index data from given buffer
  *
  * This returns a pointer to a struct delta_index that should be passed to
  * subsequent create_delta() calls, or to free_delta_index().  A NULL pointer
@@ -20,22 +19,18 @@ struct git_delta_index;
  * before free_delta_index() is called.  The returned pointer must be freed
  * using free_delta_index().
  */
-extern struct git_delta_index *git_delta_create_index(
+extern int git_delta_index_init(
-	const void *buf, unsigned long bufsize);
+	git_delta_index **out, const void *buf, size_t bufsize);
 
 /*
- * free_delta_index: free the index created by create_delta_index()
+ * Free the index created by git_delta_index_init()
- *
- * Given pointer must be what create_delta_index() returned, or NULL.
  */
-extern void git_delta_free_index(struct git_delta_index *index);
+extern void git_delta_index_free(git_delta_index *index);
 
 /*
- * sizeof_delta_index: returns memory usage of delta index
+ * Returns memory usage of delta index.
- *
- * Given pointer must be what create_delta_index() returned, or NULL.
  */
-extern unsigned long git_delta_sizeof_index(struct git_delta_index *index);
+extern size_t git_delta_index_size(git_delta_index *index);
 
 /*
  * create_delta: create a delta from given index for the given buffer
@@ -47,71 +42,50 @@ extern unsigned long git_delta_sizeof_index(struct git_delta_index *index);
  * returned and *delta_size is updated with its size.  The returned buffer
  * must be freed by the caller.
  */
-extern void *git_delta_create(
+extern int git_delta_create_from_index(
+	void **out,
+	size_t *out_size,
 	const struct git_delta_index *index,
 	const void *buf,
-	unsigned long bufsize,
+	size_t bufsize,
-	unsigned long *delta_size,
+	size_t max_delta_size);
-	unsigned long max_delta_size);
 
 /*
  * diff_delta: create a delta from source buffer to target buffer
  *
  * If max_delta_size is non-zero and the resulting delta is to be larger
- * than max_delta_size then NULL is returned.  On success, a non-NULL
+ * than max_delta_size then GIT_EBUFS is returned.  On success, a non-NULL
  * pointer to the buffer with the delta data is returned and *delta_size is
  * updated with its size.  The returned buffer must be freed by the caller.
  */
-GIT_INLINE(void *) git_delta(
+GIT_INLINE(int) git_delta(
-	const void *src_buf, unsigned long src_bufsize,
+	void **out, size_t *out_len,
-	const void *trg_buf, unsigned long trg_bufsize,
+	const void *src_buf, size_t src_bufsize,
-	unsigned long *delta_size,
+	const void *trg_buf, size_t trg_bufsize,
-	unsigned long max_delta_size)
+	size_t max_delta_size)
 {
-	struct git_delta_index *index = git_delta_create_index(src_buf, src_bufsize);
+	git_delta_index *index;
+	int error = 0;
+
+	*out = NULL;
+	*out_len = 0;
+
+	if ((error = git_delta_index_init(&index, src_buf, src_bufsize)) < 0)
+		return error;
+
 	if (index) {
-		void *delta = git_delta_create(
+		error = git_delta_create_from_index(out, out_len,
-			index, trg_buf, trg_bufsize, delta_size, max_delta_size);
+			index, trg_buf, trg_bufsize, max_delta_size);
-		git_delta_free_index(index);
+
-		return delta;
+		git_delta_index_free(index);
-	}
-	return NULL;
 	}
 
-/*
+	return error;
- * patch_delta: recreate target buffer given source buffer and delta data
+}
- *
- * On success, a non-NULL pointer to the target buffer is returned and
- * *trg_bufsize is updated with its size.  On failure a NULL pointer is
- * returned.  The returned buffer must be freed by the caller.
- */
-extern void *git_delta_patch(
-	const void *src_buf, unsigned long src_size,
-	const void *delta_buf, unsigned long delta_size,
-	unsigned long *dst_size);
 
 /* the smallest possible delta size is 4 bytes */
 #define GIT_DELTA_SIZE_MIN	4
 
-/*
- * This must be called twice on the delta data buffer, first to get the
- * expected source buffer size, and again to get the target buffer size.
- */
-GIT_INLINE(unsigned long) git_delta_get_hdr_size(
-	const unsigned char **datap, const unsigned char *top)
-{
-	const unsigned char *data = *datap;
-	unsigned long cmd, size = 0;
-	int i = 0;
-	do {
-		cmd = *data++;
-		size |= (cmd & 0x7f) << i;
-		i += 7;
-	} while (cmd & 0x80 && data < top);
-	*datap = data;
-	return size;
-}
-
 /**
 * Apply a git binary delta to recover the original content.
 * The caller is responsible for freeing the returned buffer.

src/diff_patch.c

@@ -270,21 +270,25 @@ static int create_binary(
 	}
 
 	if (a_datalen && b_datalen) {
-		void *delta_data = git_delta(
+		void *delta_data;
-			a_data, (unsigned long)a_datalen,
-			b_data, (unsigned long)b_datalen,
-			&delta_data_len, (unsigned long)deflate.size);
 
-		if (delta_data) {
+		error = git_delta(&delta_data, &delta_data_len,
+			a_data, a_datalen,
+			b_data, b_datalen,
+			deflate.size);
+
+		if (error == 0) {
 			error = git_zstream_deflatebuf(
 				&delta, delta_data, (size_t)delta_data_len);
 
 			git__free(delta_data);
+		} else if (error == GIT_EBUFS) {
+			error = 0;
+		}
 
 		if (error < 0)
 			goto done;
 	}
-	}
 
 	if (delta.size && delta.size < deflate.size) {
 		*out_type = GIT_DIFF_BINARY_DELTA;

src/pack-objects.c

@@ -274,6 +274,7 @@ static int get_delta(void **out, git_odb *odb, git_pobject *po)
 	git_odb_object *src = NULL, *trg = NULL;
 	unsigned long delta_size;
 	void *delta_buf;
+	int error;
 
 	*out = NULL;
 
@@ -281,12 +282,15 @@ static int get_delta(void **out, git_odb *odb, git_pobject *po)
 	    git_odb_read(&trg, odb, &po->id) < 0)
 		goto on_error;
 
-	delta_buf = git_delta(
+	error = git_delta(&delta_buf, &delta_size,
-		git_odb_object_data(src), (unsigned long)git_odb_object_size(src),
+		git_odb_object_data(src), git_odb_object_size(src),
-		git_odb_object_data(trg), (unsigned long)git_odb_object_size(trg),
+		git_odb_object_data(trg), git_odb_object_size(trg),
-		&delta_size, 0);
+		0);
 
-	if (!delta_buf || delta_size != po->delta_size) {
+	if (error < 0 && error != GIT_EBUFS)
+		goto on_error;
+
+	if (error == GIT_EBUFS || delta_size != po->delta_size) {
 		giterr_set(GITERR_INVALID, "Delta size changed");
 		goto on_error;
 	}
@@ -815,16 +819,14 @@ static int try_delta(git_packbuilder *pb, struct unpacked *trg,
 		*mem_usage += sz;
 	}
 	if (!src->index) {
-		src->index = git_delta_create_index(src->data, src_size);
+		if (git_delta_index_init(&src->index, src->data, src_size) < 0)
-		if (!src->index)
 			return 0; /* suboptimal pack - out of memory */
 
-		*mem_usage += git_delta_sizeof_index(src->index);
+		*mem_usage += git_delta_index_size(src->index);
 	}
 
-	delta_buf = git_delta_create(src->index, trg->data, trg_size,
+	if (git_delta_create_from_index(&delta_buf, &delta_size, src->index, trg->data, trg_size,
-				     &delta_size, max_size);
+		max_size) < 0)
-	if (!delta_buf)
 		return 0;
 
 	if (trg_object->delta) {
@@ -885,9 +887,14 @@ static unsigned int check_delta_limit(git_pobject *me, unsigned int n)
 
 static unsigned long free_unpacked(struct unpacked *n)
 {
-	unsigned long freed_mem = git_delta_sizeof_index(n->index);
+	unsigned long freed_mem = 0;
-	git_delta_free_index(n->index);
+	
+	if (n->index) {
+		freed_mem += git_delta_index_size(n->index);
+		git_delta_index_free(n->index);
+	}
 	n->index = NULL;
+
 	if (n->data) {
 		freed_mem += (unsigned long)n->object->size;
 		git__free(n->data);