Commit 38eef6113d8523abfe6746bf727cee2651398ad3

Russell Belfer 2013-04-16T14:19:27

Make indexer use shared packfile open code The indexer was creating a packfile object separately from the code in pack.c which was a problem since I put a call to git_mutex_init into just pack.c. This commit updates the pack function for creating a new pack object (i.e. git_packfile_check()) so that it can be used in both places and then makes indexer.c use the shared initialization routine. There are also a few minor formatting and warning message fixes.

diff --git a/src/indexer.c b/src/indexer.c
index 2cfbd3a..50a9d3a 100644
--- a/src/indexer.c
+++ b/src/indexer.c
@@ -60,36 +60,19 @@ const git_oid *git_indexer_stream_hash(const git_indexer_stream *idx)
 
 static int open_pack(struct git_pack_file **out, const char *filename)
 {
-	size_t namelen;
 	struct git_pack_file *pack;
-	struct stat st;
-	int fd;
 
-	namelen = strlen(filename);
-	pack = git__calloc(1, sizeof(struct git_pack_file) + namelen + 1);
-	GITERR_CHECK_ALLOC(pack);
-
-	memcpy(pack->pack_name, filename, namelen + 1);
-
-	if (p_stat(filename, &st) < 0) {
-		giterr_set(GITERR_OS, "Failed to stat packfile.");
-		goto cleanup;
-	}
+	if (git_packfile_check(&pack, filename) < 0)
+		return -1;
 
-	if ((fd = p_open(pack->pack_name, O_RDONLY)) < 0) {
+	if ((pack->mwf.fd = p_open(pack->pack_name, O_RDONLY)) < 0) {
 		giterr_set(GITERR_OS, "Failed to open packfile.");
-		goto cleanup;
+		git_packfile_free(pack);
+		return -1;
 	}
 
-	pack->mwf.fd = fd;
-	pack->mwf.size = (git_off_t)st.st_size;
-
 	*out = pack;
 	return 0;
-
-cleanup:
-	git__free(pack);
-	return -1;
 }
 
 static int parse_header(struct git_pack_header *hdr, struct git_pack_file *pack)
@@ -391,7 +374,7 @@ int git_indexer_stream_add(git_indexer_stream *idx, const void *data, size_t siz
 {
 	int error = -1;
 	struct git_pack_header hdr;
-	size_t processed; 
+	size_t processed;
 	git_mwindow_file *mwf = &idx->pack->mwf;
 
 	assert(idx && data && stats);
@@ -404,7 +387,6 @@ int git_indexer_stream_add(git_indexer_stream *idx, const void *data, size_t siz
 	/* Make sure we set the new size of the pack */
 	if (idx->opened_pack) {
 		idx->pack->mwf.size += size;
-		//printf("\nadding %zu for %zu\n", size, idx->pack->mwf.size);
 	} else {
 		if (open_pack(&idx->pack, idx->pack_file.path_lock) < 0)
 			return -1;
diff --git a/src/pack.c b/src/pack.c
index 3a2e7fb..8e8a01a 100644
--- a/src/pack.c
+++ b/src/pack.c
@@ -794,19 +794,6 @@ git_off_t get_delta_base(
  *
  ***********************************************************/
 
-static struct git_pack_file *packfile_alloc(size_t extra)
-{
-	struct git_pack_file *p = git__calloc(1, sizeof(*p) + extra);
-	if (!p)
-		return NULL;
-
-	p->mwf.fd = -1;
-	git_mutex_init(&p->lock);
-
-	return p;
-}
-
-
 void git_packfile_free(struct git_pack_file *p)
 {
 	if (!p)
@@ -902,28 +889,33 @@ int git_packfile_check(struct git_pack_file **pack_out, const char *path)
 {
 	struct stat st;
 	struct git_pack_file *p;
-	size_t path_len;
+	size_t path_len = path ? strlen(path) : 0;
 
 	*pack_out = NULL;
 
-	if (!path || (path_len = strlen(path)) <= strlen(".idx"))
+	if (path_len < strlen(".idx"))
 		return git_odb__error_notfound("invalid packfile path", NULL);
 
-	p = packfile_alloc(path_len + 2);
+	p = git__calloc(1, sizeof(*p) + path_len + 2);
 	GITERR_CHECK_ALLOC(p);
 
+	memcpy(p->pack_name, path, path_len + 1);
+
 	/*
 	 * Make sure a corresponding .pack file exists and that
 	 * the index looks sane.
 	 */
-	path_len -= strlen(".idx");
-	memcpy(p->pack_name, path, path_len);
+	if (git__suffixcmp(path, ".idx") == 0) {
+		size_t root_len = path_len - strlen(".idx");
+
+		memcpy(p->pack_name + root_len, ".keep", sizeof(".keep"));
+		if (git_path_exists(p->pack_name) == true)
+			p->pack_keep = 1;
 
-	memcpy(p->pack_name + path_len, ".keep", sizeof(".keep"));
-	if (git_path_exists(p->pack_name) == true)
-		p->pack_keep = 1;
+		memcpy(p->pack_name + root_len, ".pack", sizeof(".pack"));
+		path_len = path_len - strlen(".idx") + strlen(".pack");
+	}
 
-	memcpy(p->pack_name + path_len, ".pack", sizeof(".pack"));
 	if (p_stat(p->pack_name, &st) < 0 || !S_ISREG(st.st_mode)) {
 		git__free(p);
 		return git_odb__error_notfound("packfile not found", NULL);
@@ -932,10 +924,13 @@ int git_packfile_check(struct git_pack_file **pack_out, const char *path)
 	/* ok, it looks sane as far as we can check without
 	 * actually mapping the pack file.
 	 */
+	p->mwf.fd = -1;
 	p->mwf.size = st.st_size;
 	p->pack_local = 1;
 	p->mtime = (git_time_t)st.st_mtime;
 
+	git_mutex_init(&p->lock);
+
 	/* see if we can parse the sha1 oid in the packfile name */
 	if (path_len < 40 ||
 		git_oid_fromstr(&p->sha1, path + path_len - GIT_OID_HEXSZ) < 0)
diff --git a/src/thread-utils.h b/src/thread-utils.h
index e53a60c..dafe70a 100644
--- a/src/thread-utils.h
+++ b/src/thread-utils.h
@@ -71,7 +71,7 @@ GIT_INLINE(int) git_atomic_dec(git_atomic *a)
 GIT_INLINE(void *) git___compare_and_swap(
 	volatile void **ptr, void *oldval, void *newval)
 {
-	void *foundval;
+	volatile void *foundval;
 #if defined(GIT_WIN32)
 	foundval = InterlockedCompareExchangePointer(ptr, newval, oldval);
 #elif defined(__GNUC__)
diff --git a/src/win32/pthread.c b/src/win32/pthread.c
index c78213f..232709e 100644
--- a/src/win32/pthread.c
+++ b/src/win32/pthread.c
@@ -33,8 +33,9 @@ int pthread_join(pthread_t thread, void **value_ptr)
 	return -1;
 }
 
-int pthread_mutex_init(pthread_mutex_t *GIT_RESTRICT mutex,
-						const pthread_mutexattr_t *GIT_RESTRICT mutexattr)
+int pthread_mutex_init(
+	pthread_mutex_t *GIT_RESTRICT mutex,
+	const pthread_mutexattr_t *GIT_RESTRICT mutexattr)
 {
 	GIT_UNUSED(mutexattr);
 	InitializeCriticalSection(mutex);
diff --git a/src/win32/pthread.h b/src/win32/pthread.h
index a219a01..8277ecf 100644
--- a/src/win32/pthread.h
+++ b/src/win32/pthread.h
@@ -25,13 +25,16 @@ typedef HANDLE pthread_cond_t;
 
 #define PTHREAD_MUTEX_INITIALIZER {(void*)-1};
 
-int pthread_create(pthread_t *GIT_RESTRICT,
-					const pthread_attr_t *GIT_RESTRICT,
-					void *(*start_routine)(void*), void *__restrict);
+int pthread_create(
+	pthread_t *GIT_RESTRICT,
+	const pthread_attr_t *GIT_RESTRICT,
+	void *(*start_routine)(void*),
+	void *__restrict);
 
 int pthread_join(pthread_t, void **);
 
-int pthread_mutex_init(pthread_mutex_t *GIT_RESTRICT, const pthread_mutexattr_t *GIT_RESTRICT);
+int pthread_mutex_init(
+	pthread_mutex_t *GIT_RESTRICT, const pthread_mutexattr_t *GIT_RESTRICT);
 int pthread_mutex_destroy(pthread_mutex_t *);
 int pthread_mutex_lock(pthread_mutex_t *);
 int pthread_mutex_unlock(pthread_mutex_t *);
diff --git a/tests-clar/object/cache.c b/tests-clar/object/cache.c
index 8121247..ed13a6f 100644
--- a/tests-clar/object/cache.c
+++ b/tests-clar/object/cache.c
@@ -213,16 +213,16 @@ void test_object_cache__threadmania(void)
 			fn = (th & 1) ? cache_parsed : cache_raw;
 
 #ifdef GIT_THREADS
-			git_thread_create(&t[th], NULL, fn, data);
+			cl_git_pass(git_thread_create(&t[th], NULL, fn, data));
 #else
-			fn(data);
+			cl_assert(fn(data) == data);
 			git__free(data);
 #endif
 		}
 
 #ifdef GIT_THREADS
 		for (th = 0; th < THREADCOUNT; ++th) {
-			git_thread_join(t[th], &data);
+			cl_git_pass(git_thread_join(t[th], &data));
 			cl_assert_equal_i(th, ((int *)data)[0]);
 			git__free(data);
 		}