Clean up GIT_UNUSED macros on all platforms

It turns out that commit 31e9cfc4cbcaf1b38cdd3dbe3282a8f57e5366a5
did not fix the GIT_USUSED behavior on all platforms.  This commit
walks through and really cleans things up more thoroughly, getting
rid of the unnecessary stuff.

To remove the use of some GIT_UNUSED, I ended up adding a couple
of new iterators for hashtables that allow you to iterator just
over keys or just over values.

In making this change, I found a bug in the clar tests (where we
were doing *count++ but meant to do (*count)++ to increment the
value).  I fixed that but then found the test failing because it
was not really using an empty repo.  So, I took some of the code
that I wrote for iterator testing and moved it to clar_helpers.c,
then made use of that to make it easier to open fixtures on a
per test basis even within a single test file.

15 files changed, 61 insertions, 70 deletions

diff --git a/src/attr.c b/src/attr.c
index a7c65f94c..603498df2 100644
--- a/src/attr.c
+++ b/src/attr.c
@@ -408,10 +408,9 @@ void git_attr_cache_flush(
 		return;
 
 	if (repo->attrcache.files) {
-		const void *GIT_UNUSED(name);
 		git_attr_file *file;
 
-		GIT_HASHTABLE_FOREACH(repo->attrcache.files, name, file,
+		GIT_HASHTABLE_FOREACH_VALUE(repo->attrcache.files, file,
 			git_attr_file__free(file));
 
 		git_hashtable_free(repo->attrcache.files);
@@ -419,10 +418,9 @@ void git_attr_cache_flush(
 	}
 
 	if (repo->attrcache.macros) {
-		const void *GIT_UNUSED(name);
 		git_attr_rule *rule;
 
-		GIT_HASHTABLE_FOREACH(repo->attrcache.macros, name, rule,
+		GIT_HASHTABLE_FOREACH_VALUE(repo->attrcache.macros, rule,
 			git_attr_rule__free(rule));
 
 		git_hashtable_free(repo->attrcache.macros);
diff --git a/src/cc-compat.h b/src/cc-compat.h
index bbccd1f55..3df36b61f 100644
--- a/src/cc-compat.h
+++ b/src/cc-compat.h
@@ -33,8 +33,7 @@
 #	define GIT_TYPEOF(x)
 #endif
 
-#define GIT_UNUSED(x) x
-#define GIT_UNUSED_ARG(x) ((void)(x))
+#define GIT_UNUSED(x) ((void)(x))
 
 /* Define the printf format specifer to use for size_t output */
 #if defined(_MSC_VER) || defined(__MINGW32__)
diff --git a/src/config_file.c b/src/config_file.c
index ce76493c7..3c7c593ec 100644
--- a/src/config_file.c
+++ b/src/config_file.c
@@ -125,13 +125,12 @@ static int normalize_name(const char *in, char **out)
 
 static void free_vars(git_hashtable *values)
 {
-	const char *GIT_UNUSED(_unused) = NULL;
 	cvar_t *var = NULL;
 
 	if (values == NULL)
 		return;
 
-	GIT_HASHTABLE_FOREACH(values, _unused, var,
+	GIT_HASHTABLE_FOREACH_VALUE(values, var,
 	      do {
 		      cvar_t *next = CVAR_LIST_NEXT(var);
 		      cvar_free(var);
diff --git a/src/diff.c b/src/diff.c
index 9e4105571..dcc0aef6a 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -325,7 +325,7 @@ static int maybe_modified(
 	int error = GIT_SUCCESS;
 	git_oid noid, *use_noid = NULL;
 
-	GIT_UNUSED_ARG(old);
+	GIT_UNUSED(old);
 
 	/* support "assume unchanged" & "skip worktree" bits */
 	if ((oitem->flags_extended & GIT_IDXENTRY_INTENT_TO_ADD) != 0 ||
diff --git a/src/diff_output.c b/src/diff_output.c
index ac60e9822..b800be933 100644
--- a/src/diff_output.c
+++ b/src/diff_output.c
@@ -161,7 +161,7 @@ static int file_is_binary_by_content(
 	git_map *old_data,
 	git_map *new_data)
 {
-	GIT_UNUSED_ARG(diff);
+	GIT_UNUSED(diff);
 
 	if ((delta->old.flags & BINARY_DIFF_FLAGS) == 0) {
 		size_t search_len = min(old_data->len, 4000);
@@ -448,7 +448,7 @@ static int print_compact(void *data, git_diff_delta *delta, float progress)
 	diff_print_info *pi = data;
 	char code, old_suffix, new_suffix;
 
-	GIT_UNUSED_ARG(progress);
+	GIT_UNUSED(progress);
 
 	switch (delta->status) {
 	case GIT_STATUS_ADDED: code = 'A'; break;
@@ -546,7 +546,7 @@ static int print_patch_file(void *data, git_diff_delta *delta, float progress)
 	const char *newpfx = pi->diff->opts.dst_prefix;
 	const char *newpath = delta->new.path;
 
-	GIT_UNUSED_ARG(progress);
+	GIT_UNUSED(progress);
 
 	git_buf_clear(pi->buf);
 	git_buf_printf(pi->buf, "diff --git %s%s %s%s\n", oldpfx, delta->old.path, newpfx, delta->new.path);
@@ -593,8 +593,8 @@ static int print_patch_hunk(
 {
 	diff_print_info *pi = data;
 
-	GIT_UNUSED_ARG(d);
-	GIT_UNUSED_ARG(r);
+	GIT_UNUSED(d);
+	GIT_UNUSED(r);
 
 	git_buf_clear(pi->buf);
 
@@ -613,7 +613,7 @@ static int print_patch_line(
 {
 	diff_print_info *pi = data;
 
-	GIT_UNUSED_ARG(delta);
+	GIT_UNUSED(delta);
 
 	git_buf_clear(pi->buf);
 
diff --git a/src/hashtable.h b/src/hashtable.h
index f6fbb8585..e09965965 100644
--- a/src/hashtable.h
+++ b/src/hashtable.h
@@ -65,20 +65,20 @@ GIT_INLINE(int) git_hashtable_insert(git_hashtable *h, const void *key, void *va
 
 #define git_hashtable_node_at(nodes, pos) ((git_hashtable_node *)(&nodes[pos]))
 
-#define GIT_HASHTABLE_FOREACH(self, pkey, pvalue, code) {\
-	git_hashtable *_self = (self);\
-	git_hashtable_node *_nodes = _self->nodes;\
-	unsigned int _i, _size = _self->size;\
-	for (_i = 0; _i < _size; _i ++) {\
-		git_hashtable_node *_node = git_hashtable_node_at(_nodes, _i);\
-		if (_node->key)\
-		{\
-			pkey = _node->key;\
-			pvalue = _node->value;\
-			code;\
-		}\
-	}\
-}
+#define GIT_HASHTABLE__FOREACH(self,block) { \
+	unsigned int _c; \
+	git_hashtable_node *_n = (self)->nodes; \
+	for (_c = (self)->size; _c > 0; _c--, _n++)	{ \
+		if (!_n->key) continue; block } }
+
+#define GIT_HASHTABLE_FOREACH(self, pkey, pvalue, code)\
+	GIT_HASHTABLE__FOREACH(self,{(pkey)=_n->key;(pvalue)=_n->value;code;})
+
+#define GIT_HASHTABLE_FOREACH_KEY(self, pkey, code)\
+	GIT_HASHTABLE__FOREACH(self,{(pkey)=_n->key;code;})
+
+#define GIT_HASHTABLE_FOREACH_VALUE(self, pvalue, code)\
+	GIT_HASHTABLE__FOREACH(self,{(pvalue)=_n->value;code;})
 
 #define GIT_HASHTABLE_FOREACH_DELETE() {\
 	_node->key = NULL; _node->value = NULL; _self->key_count--;\
diff --git a/src/odb_pack.c b/src/odb_pack.c
index 9e1004eb8..249144a3a 100644
--- a/src/odb_pack.c
+++ b/src/odb_pack.c
@@ -159,9 +159,9 @@ static int pack_entry_find_prefix(struct git_pack_entry *e,
  *
  ***********************************************************/
 
-GIT_INLINE(void) pack_window_free_all(struct pack_backend *GIT_UNUSED(backend), struct git_pack_file *p)
+GIT_INLINE(void) pack_window_free_all(struct pack_backend *backend, struct git_pack_file *p)
 {
-	GIT_UNUSED_ARG(backend);
+	GIT_UNUSED(backend);
 	git_mwindow_free_all(&p->mwf);
 }
 
diff --git a/src/pkt.c b/src/pkt.c
index df972e72a..51da55de1 100644
--- a/src/pkt.c
+++ b/src/pkt.c
@@ -41,11 +41,11 @@ static int flush_pkt(git_pkt **out)
 }
 
 /* the rest of the line will be useful for multi_ack */
-static int ack_pkt(git_pkt **out, const char *GIT_UNUSED(line), size_t GIT_UNUSED(len))
+static int ack_pkt(git_pkt **out, const char *line, size_t len)
 {
 	git_pkt *pkt;
-	GIT_UNUSED_ARG(line);
-	GIT_UNUSED_ARG(len);
+	GIT_UNUSED(line);
+	GIT_UNUSED(len);
 
 	pkt = git__malloc(sizeof(git_pkt));
 	if (pkt == NULL)
diff --git a/src/refs.c b/src/refs.c
index 2e1d92da2..f3388bf53 100644
--- a/src/refs.c
+++ b/src/refs.c
@@ -105,7 +105,7 @@ static int reference_alloc(
 
 	reference->name = git__strdup(name);
 	if (reference->name == NULL) {
-		free(reference);
+		git__free(reference);
 		return GIT_ENOMEM;
 	}
 
@@ -222,7 +222,7 @@ static int loose_lookup(git_reference *ref)
 		return GIT_SUCCESS;
 
 	if (ref->flags & GIT_REF_SYMBOLIC) {
-		free(ref->target.symbolic);
+		git__free(ref->target.symbolic);
 		ref->target.symbolic = NULL;
 	}
 
@@ -278,7 +278,8 @@ static int loose_lookup_to_packfile(
 
 cleanup:
 	git_buf_free(&ref_file);
-	free(ref);
+	git__free(ref);
+
 	return git__rethrow(error, "Failed to lookup loose reference");
 }
 
@@ -420,7 +421,7 @@ static int packed_parse_oid(
 	return GIT_SUCCESS;
 
 cleanup:
-	free(ref);
+	git__free(ref);
 	return git__rethrow(error, "Failed to parse OID of packed reference");
 }
 
@@ -495,7 +496,7 @@ static int packed_load(git_repository *repo)
 
 		error = git_hashtable_insert(ref_cache->packfile, ref->name, ref);
 		if (error < GIT_SUCCESS) {
-			free(ref);
+			git__free(ref);
 			goto cleanup;
 		}
 	}
@@ -560,12 +561,12 @@ static int _dirent_loose_load(void *data, git_buf *full_path)
 		if (git_hashtable_insert2(
 			repository->references.packfile,
 			ref->name, ref, &old_ref) < GIT_SUCCESS) {
-			free(ref);
+			git__free(ref);
 			return GIT_ENOMEM;
 		}
 
 		if (old_ref != NULL)
-			free(old_ref);
+			git__free(old_ref);
 	}
 
 	return error == GIT_SUCCESS ?
@@ -773,9 +774,8 @@ static int packed_write(git_repository *repo)
 	/* Load all the packfile into a vector */
 	{
 		struct packref *reference;
-		const void *GIT_UNUSED(_unused);
 
-		GIT_HASHTABLE_FOREACH(repo->references.packfile, _unused, reference,
+		GIT_HASHTABLE_FOREACH_VALUE(repo->references.packfile, reference,
 			/* cannot fail: vector already has the right size */
 			git_vector_insert(&packing_list, reference);
 		);
@@ -929,7 +929,7 @@ static int packed_lookup(git_reference *ref)
 		return GIT_SUCCESS;
 
 	if (ref->flags & GIT_REF_SYMBOLIC) {
-		free(ref->target.symbolic);
+		git__free(ref->target.symbolic);
 		ref->target.symbolic = NULL;
 	}
 
@@ -1513,12 +1513,11 @@ int git_reference_foreach(
 	/* list all the packed references first */
 	if (list_flags & GIT_REF_PACKED) {
 		const char *ref_name;
-		void *GIT_UNUSED(_unused);
 
 		if ((error = packed_load(repo)) < GIT_SUCCESS)
 			return git__rethrow(error, "Failed to list references");
 
-		GIT_HASHTABLE_FOREACH(repo->references.packfile, ref_name, _unused,
+		GIT_HASHTABLE_FOREACH_KEY(repo->references.packfile, ref_name,
 			if ((error = callback(ref_name, payload)) < GIT_SUCCESS)
 				return git__throw(error,
 					"Failed to list references. User callback failed");
@@ -1595,12 +1594,10 @@ void git_repository__refcache_free(git_refcache *refs)
 	assert(refs);
 
 	if (refs->packfile) {
-		const void *GIT_UNUSED(_unused);
 		struct packref *reference;
 
-		GIT_HASHTABLE_FOREACH(refs->packfile, _unused, reference,
-			free(reference);
-		);
+		GIT_HASHTABLE_FOREACH_VALUE(
+			refs->packfile, reference, git__free(reference));
 
 		git_hashtable_free(refs->packfile);
 	}
diff --git a/src/remote.c b/src/remote.c
index 5b442e934..52b6aacc9 100644
--- a/src/remote.c
+++ b/src/remote.c
@@ -431,13 +431,13 @@ struct cb_data {
 	regex_t *preg;
 };
 
-static int remote_list_cb(const char *name, const char *GIT_UNUSED(value), void *data_)
+static int remote_list_cb(const char *name, const char *value, void *data_)
 {
 	struct cb_data *data = (struct cb_data *)data_;
 	size_t nmatch = 2;
 	regmatch_t pmatch[2];
 	int error;
-	GIT_UNUSED_ARG(value);
+	GIT_UNUSED(value);
 
 	if (!regexec(data->preg, name, nmatch, pmatch, 0)) {
 		char *remote_name = git__strndup(&name[pmatch[1].rm_so], pmatch[1].rm_eo - pmatch[1].rm_so);
diff --git a/src/revwalk.c b/src/revwalk.c
index cd971b5d9..997771f08 100644
--- a/src/revwalk.c
+++ b/src/revwalk.c
@@ -590,7 +590,6 @@ int git_revwalk_new(git_revwalk **revwalk_out, git_repository *repo)
 void git_revwalk_free(git_revwalk *walk)
 {
 	unsigned int i;
-	const void *GIT_UNUSED(_unused);
 	commit_object *commit;
 
 	if (walk == NULL)
@@ -602,7 +601,7 @@ void git_revwalk_free(git_revwalk *walk)
 	/* if the parent has more than PARENTS_PER_COMMIT parents,
 	 * we had to allocate a separate array for those parents.
 	 * make sure it's being free'd */
-	GIT_HASHTABLE_FOREACH(walk->commits, _unused, commit, {
+	GIT_HASHTABLE_FOREACH_VALUE(walk->commits, commit, {
 		if (commit->out_degree > PARENTS_PER_COMMIT)
 			git__free(commit->parents);
 	});
@@ -669,12 +668,11 @@ int git_revwalk_next(git_oid *oid, git_revwalk *walk)
 
 void git_revwalk_reset(git_revwalk *walk)
 {
-	const void *GIT_UNUSED(_unused);
 	commit_object *commit;
 
 	assert(walk);
 
-	GIT_HASHTABLE_FOREACH(walk->commits, _unused, commit,
+	GIT_HASHTABLE_FOREACH_VALUE(walk->commits, commit,
 		commit->seen = 0;
 		commit->in_degree = 0;
 		commit->topo_delay = 0;
diff --git a/src/transport.c b/src/transport.c
index 672eb6e8a..785ddc35d 100644
--- a/src/transport.c
+++ b/src/transport.c
@@ -43,9 +43,9 @@ static git_transport_cb transport_find_fn(const char *url)
  * Public API *
  **************/
 
-int git_transport_dummy(git_transport **GIT_UNUSED(transport))
+int git_transport_dummy(git_transport **transport)
 {
-	GIT_UNUSED_ARG(transport);
+	GIT_UNUSED(transport);
 	return git__throw(GIT_ENOTIMPLEMENTED, "This protocol isn't implemented. Sorry");
 }
 
diff --git a/src/transports/local.c b/src/transports/local.c
index 1dfc8ed2e..eb24db0fd 100644
--- a/src/transports/local.c
+++ b/src/transports/local.c
@@ -154,7 +154,7 @@ static int local_ls(git_transport *transport, git_headlist_cb list_cb, void *pay
  * Try to open the url as a git directory. The direction doesn't
  * matter in this case because we're calulating the heads ourselves.
  */
-static int local_connect(git_transport *transport, int GIT_UNUSED(direction))
+static int local_connect(git_transport *transport, int direction)
 {
 	git_repository *repo;
 	int error;
@@ -162,7 +162,7 @@ static int local_connect(git_transport *transport, int GIT_UNUSED(direction))
 	const char *path;
 	git_buf buf = GIT_BUF_INIT;
 
-	GIT_UNUSED_ARG(direction);
+	GIT_UNUSED(direction);
 
 	/* The repo layer doesn't want the prefix */
 	if (!git__prefixcmp(transport->url, "file://")) {
@@ -194,7 +194,7 @@ static int local_connect(git_transport *transport, int GIT_UNUSED(direction))
 	return GIT_SUCCESS;
 }
 
-static int local_close(git_transport *GIT_UNUSED(transport))
+static int local_close(git_transport *transport)
 {
 	transport_local *t = (transport_local *)transport;
 
diff --git a/src/win32/posix.h b/src/win32/posix.h
index f4c1c121e..60adc9666 100644
--- a/src/win32/posix.h
+++ b/src/win32/posix.h
@@ -11,20 +11,20 @@
 #include "fnmatch.h"
 #include "utf-conv.h"
 
-GIT_INLINE(int) p_link(const char *GIT_UNUSED(old), const char *GIT_UNUSED(new))
+GIT_INLINE(int) p_link(const char *old, const char *new)
 {
-	GIT_UNUSED_ARG(old);
-	GIT_UNUSED_ARG(new);
+	GIT_UNUSED(old);
+	GIT_UNUSED(new);
 	errno = ENOSYS;
 	return -1;
 }
 
-GIT_INLINE(int) p_mkdir(const char *path, mode_t GIT_UNUSED(mode))
+GIT_INLINE(int) p_mkdir(const char *path, mode_t mode)
 {
 	wchar_t* buf = gitwin_to_utf16(path);
 	int ret = _wmkdir(buf);
 
-	GIT_UNUSED_ARG(mode);
+	GIT_UNUSED(mode);
 
 	git__free(buf);
 	return ret;
diff --git a/src/win32/pthread.c b/src/win32/pthread.c
index cbce639c0..3db536848 100644
--- a/src/win32/pthread.c
+++ b/src/win32/pthread.c
@@ -8,10 +8,10 @@
 #include "pthread.h"
 
 int pthread_create(pthread_t *GIT_RESTRICT thread,
-					const pthread_attr_t *GIT_RESTRICT GIT_UNUSED(attr),
+					const pthread_attr_t *GIT_RESTRICT attr,
 					void *(*start_routine)(void*), void *GIT_RESTRICT arg)
 {
-	GIT_UNUSED_ARG(attr);
+	GIT_UNUSED(attr);
 	*thread = (pthread_t) CreateThread(NULL, 0, (LPTHREAD_START_ROUTINE)start_routine, arg, 0, NULL);
 	return *thread ? GIT_SUCCESS : git__throw(GIT_EOSERR, "Failed to create pthread");
 }
@@ -26,9 +26,9 @@ int pthread_join(pthread_t thread, void **value_ptr)
 }
 
 int pthread_mutex_init(pthread_mutex_t *GIT_RESTRICT mutex,
-						const pthread_mutexattr_t *GIT_RESTRICT GIT_UNUSED(mutexattr))
+						const pthread_mutexattr_t *GIT_RESTRICT mutexattr)
 {
-	GIT_UNUSED_ARG(mutexattr);
+	GIT_UNUSED(mutexattr);
 	InitializeCriticalSection(mutex);
 	return 0;
 }