Merge pull request #5552 from libgit2/pks/small-fixes

Random code cleanups and fixes

src/checkout.c

@@ -44,7 +44,6 @@ enum {
 	CHECKOUT_ACTION__REMOVE_CONFLICT = 16,
 	CHECKOUT_ACTION__UPDATE_CONFLICT = 32,
 	CHECKOUT_ACTION__MAX = 32,
-	CHECKOUT_ACTION__DEFER_REMOVE = 64,
 	CHECKOUT_ACTION__REMOVE_AND_UPDATE =
 		(CHECKOUT_ACTION__UPDATE_BLOB | CHECKOUT_ACTION__REMOVE),
 };
@@ -196,7 +195,7 @@ static bool checkout_is_workdir_modified(
 		}
 
 		if (git_submodule_status(&sm_status, data->repo, wditem->path, GIT_SUBMODULE_IGNORE_UNSPECIFIED) < 0 ||
-			GIT_SUBMODULE_STATUS_IS_WD_DIRTY(sm_status))
+		    GIT_SUBMODULE_STATUS_IS_WD_DIRTY(sm_status))
 			rval = true;
 		else if ((sm_oid = git_submodule_wd_id(sm)) == NULL)
 			rval = false;
@@ -274,9 +273,8 @@ static int checkout_action_common(
 
 		/* if the file is on disk and doesn't match our mode, force update */
 		if (wd &&
-			GIT_PERMS_IS_EXEC(wd->mode) !=
-			GIT_PERMS_IS_EXEC(delta->new_file.mode))
-				*action |= CHECKOUT_ACTION__REMOVE;
+		    GIT_PERMS_IS_EXEC(wd->mode) != GIT_PERMS_IS_EXEC(delta->new_file.mode))
+			*action |= CHECKOUT_ACTION__REMOVE;
 
 		notify = GIT_CHECKOUT_NOTIFY_UPDATED;
 	}
@@ -800,7 +798,7 @@ static int checkout_conflictdata_cmp(const void *a, const void *b)
 	int diff;
 
 	if ((diff = checkout_idxentry_cmp(ca->ancestor, cb->ancestor)) == 0 &&
-		(diff = checkout_idxentry_cmp(ca->ours, cb->theirs)) == 0)
+	    (diff = checkout_idxentry_cmp(ca->ours, cb->theirs)) == 0)
 		diff = checkout_idxentry_cmp(ca->theirs, cb->theirs);
 
 	return diff;
@@ -1179,7 +1177,7 @@ static int checkout_conflicts_mark_directoryfile(
 	/* Find d/f conflicts */
 	git_vector_foreach(&data->update_conflicts, i, conflict) {
 		if ((conflict->ours && conflict->theirs) ||
-			(!conflict->ours && !conflict->theirs))
+		    (!conflict->ours && !conflict->theirs))
 			continue;
 
 		path = conflict->ours ?
@@ -1228,8 +1226,8 @@ static int checkout_get_update_conflicts(
 		return 0;
 
 	if ((error = checkout_conflicts_load(data, workdir, pathspec)) < 0 ||
-		(error = checkout_conflicts_coalesce_renames(data)) < 0 ||
-		(error = checkout_conflicts_mark_directoryfile(data)) < 0)
+	    (error = checkout_conflicts_coalesce_renames(data)) < 0 ||
+	    (error = checkout_conflicts_mark_directoryfile(data)) < 0)
 		goto done;
 
 done:
@@ -1314,11 +1312,11 @@ static int checkout_get_actions(
 		return -1;
 
 	if (data->opts.paths.count > 0 &&
-		git_pathspec__vinit(&pathspec, &data->opts.paths, &pathpool) < 0)
+	    git_pathspec__vinit(&pathspec, &data->opts.paths, &pathpool) < 0)
 		return -1;
 
 	if ((error = git_iterator_current(&wditem, workdir)) < 0 &&
-		error != GIT_ITEROVER)
+	    error != GIT_ITEROVER)
 		goto fail;
 
 	deltas = &data->diff->deltas;
@@ -1357,8 +1355,7 @@ static int checkout_get_actions(
 	counts[CHECKOUT_ACTION__REMOVE] += data->removes.length;
 
 	if (counts[CHECKOUT_ACTION__CONFLICT] > 0 &&
-		(data->strategy & GIT_CHECKOUT_ALLOW_CONFLICTS) == 0)
-	{
+	    (data->strategy & GIT_CHECKOUT_ALLOW_CONFLICTS) == 0) {
 		git_error_set(GIT_ERROR_CHECKOUT, "%"PRIuZ" %s checkout",
 			counts[CHECKOUT_ACTION__CONFLICT],
 			counts[CHECKOUT_ACTION__CONFLICT] == 1 ?
@@ -1369,7 +1366,7 @@ static int checkout_get_actions(
 
 
 	if ((error = checkout_get_remove_conflicts(data, workdir, &pathspec)) < 0 ||
-		(error = checkout_get_update_conflicts(data, workdir, &pathspec)) < 0)
+	    (error = checkout_get_update_conflicts(data, workdir, &pathspec)) < 0)
 		goto fail;
 
 	counts[CHECKOUT_ACTION__REMOVE_CONFLICT] = git_vector_length(&data->remove_conflicts);
@@ -1860,26 +1857,6 @@ static int checkout_remove_the_old(
 	return 0;
 }
 
-static int checkout_deferred_remove(git_repository *repo, const char *path)
-{
-#if 0
-	int error = git_futils_rmdir_r(
-		path, data->opts.target_directory, GIT_RMDIR_EMPTY_PARENTS);
-
-	if (error == GIT_ENOTFOUND) {
-		error = 0;
-		git_error_clear();
-	}
-
-	return error;
-#else
-	GIT_UNUSED(repo);
-	GIT_UNUSED(path);
-	assert(false);
-	return 0;
-#endif
-}
-
 static int checkout_create_the_new(
 	unsigned int *actions,
 	checkout_data *data)
@@ -1889,15 +1866,6 @@ static int checkout_create_the_new(
 	size_t i;
 
 	git_vector_foreach(&data->diff->deltas, i, delta) {
-		if (actions[i] & CHECKOUT_ACTION__DEFER_REMOVE) {
-			/* this had a blocker directory that should only be removed iff
-			 * all of the contents of the directory were safely removed
-			 */
-			if ((error = checkout_deferred_remove(
-					data->repo, delta->old_file.path)) < 0)
-				return error;
-		}
-
 		if (actions[i] & CHECKOUT_ACTION__UPDATE_BLOB && !S_ISLNK(delta->new_file.mode)) {
 			if ((error = checkout_blob(data, &delta->new_file)) < 0)
 				return error;
@@ -1922,20 +1890,10 @@ static int checkout_create_submodules(
 	unsigned int *actions,
 	checkout_data *data)
 {
-	int error = 0;
 	git_diff_delta *delta;
 	size_t i;
 
 	git_vector_foreach(&data->diff->deltas, i, delta) {
-		if (actions[i] & CHECKOUT_ACTION__DEFER_REMOVE) {
-			/* this has a blocker directory that should only be removed iff
-			 * all of the contents of the directory were safely removed
-			 */
-			if ((error = checkout_deferred_remove(
-					data->repo, delta->old_file.path)) < 0)
-				return error;
-		}
-
 		if (actions[i] & CHECKOUT_ACTION__UPDATE_SUBMODULE) {
 			int error = checkout_submodule(data, &delta->new_file);
 			if (error < 0)

src/revparse.c

@@ -537,7 +537,8 @@ static int extract_curly_braces_content(git_buf *buf, const char *spec, size_t *
 		if (spec[*pos] == '\0')
 			return GIT_EINVALIDSPEC;
 
-		git_buf_putc(buf, spec[(*pos)++]);
+		if (git_buf_putc(buf, spec[(*pos)++]) < 0)
+			return -1;
 	}
 
 	(*pos)++;
@@ -585,7 +586,7 @@ static int extract_how_many(int *n, const char *spec, size_t *pos)
 			*pos = end_ptr - spec;
 		}
 
-	} 	while (spec[(*pos)] == kind && kind == '~');
+	} while (spec[(*pos)] == kind && kind == '~');
 
 	*n = accumulated;
 

tests/refs/create.c

@@ -212,17 +212,12 @@ void test_refs_create__oid_unknown_fails_by_default(void)
 
 void test_refs_create__propagate_eexists(void)
 {
-	int error;
 	git_oid oid;
-	git_reference *ref;
 
 	/* Make sure it works for oid and for symbolic both */
-	git_oid_fromstr(&oid, current_master_tip);
-	error = git_reference_create(&ref, g_repo, current_head_target, &oid, false, NULL);
-	cl_assert(error == GIT_EEXISTS);
-
-	error = git_reference_symbolic_create(&ref, g_repo, "HEAD", current_head_target, false, NULL);
-	cl_assert(error == GIT_EEXISTS);
+	cl_git_pass(git_oid_fromstr(&oid, current_master_tip));
+	cl_git_fail_with(GIT_EEXISTS, git_reference_create(NULL, g_repo, current_head_target, &oid, false, NULL));
+	cl_git_fail_with(GIT_EEXISTS, git_reference_symbolic_create(NULL, g_repo, "HEAD", current_head_target, false, NULL));
 }
 
 void test_refs_create__existing_dir_propagates_edirectory(void)