Commit 56543a609aa7adb14b308046445ddd48f44322b7

Russell Belfer 2013-02-15T16:02:45

Clear up warnings from cppcheck The cppcheck static analyzer generates warnings for a bunch of places in the libgit2 code base. All the ones fixed in this commit are actually false positives, but I've reorganized the code to hopefully make it easier for static analysis tools to correctly understand the structure. I wouldn't do this if I felt like it was making the code harder to read or worse for humans, but in this case, these fixes don't seem too bad and will hopefully make it easier for better analysis tools to get at any real issues.

diff --git a/src/checkout.c b/src/checkout.c
index 0ce283b..59cd218 100644
--- a/src/checkout.c
+++ b/src/checkout.c
@@ -64,6 +64,7 @@ static int checkout_notify(
 {
 	git_diff_file wdfile;
 	const git_diff_file *baseline = NULL, *target = NULL, *workdir = NULL;
+	const char *path = NULL;
 
 	if (!data->opts.notify_cb)
 		return 0;
@@ -81,6 +82,8 @@ static int checkout_notify(
 		wdfile.mode = wditem->mode;
 
 		workdir = &wdfile;
+
+		path = wditem->path;
 	}
 
 	if (delta) {
@@ -101,11 +104,12 @@ static int checkout_notify(
 			baseline = &delta->old_file;
 			break;
 		}
+
+		path = delta->old_file.path;
 	}
 
 	return data->opts.notify_cb(
-		why, delta ? delta->old_file.path : wditem->path,
-		baseline, target, workdir, data->opts.notify_payload);
+		why, path, baseline, target, workdir, data->opts.notify_payload);
 }
 
 static bool checkout_is_workdir_modified(
@@ -683,7 +687,7 @@ static int blob_content_to_file(
 {
 	int error = -1, nb_filters = 0;
 	mode_t file_mode = opts->file_mode;
-	bool dont_free_filtered = false;
+	bool dont_free_filtered;
 	git_buf unfiltered = GIT_BUF_INIT, filtered = GIT_BUF_INIT;
 	git_vector filters = GIT_VECTOR_INIT;
 
diff --git a/src/diff_output.c b/src/diff_output.c
index 26b073a..88ccc9d 100644
--- a/src/diff_output.c
+++ b/src/diff_output.c
@@ -1280,14 +1280,15 @@ static void set_data_from_buffer(
 {
 	file->size = (git_off_t)buffer_len;
 	file->mode = 0644;
+	map->len   = buffer_len;
 
-	if (!buffer)
+	if (!buffer) {
 		file->flags |= GIT_DIFF_FILE_NO_DATA;
-	else
+		map->data = NULL;
+	} else {
+		map->data = (char *)buffer;
 		git_odb_hash(&file->oid, buffer, buffer_len, GIT_OBJ_BLOB);
-
-	map->len   = buffer_len;
-	map->data  = (char *)buffer;
+	}
 }
 
 typedef struct {
diff --git a/src/notes.c b/src/notes.c
index eff80bc..f5537db 100644
--- a/src/notes.c
+++ b/src/notes.c
@@ -147,7 +147,7 @@ static int manipulate_note_in_tree_r(
 		int fanout,
 		int current_error))
 {
-	int error = -1;
+	int error;
 	git_tree *subtree = NULL, *new = NULL;
 	char subtree_name[3];
 
diff --git a/src/refs.c b/src/refs.c
index 7dabfef..866c230 100644
--- a/src/refs.c
+++ b/src/refs.c
@@ -1493,7 +1493,7 @@ int git_reference_foreach(
 	/* list all the packed references first */
 	if (list_flags & GIT_REF_PACKED) {
 		const char *ref_name;
-		void *ref;
+		void *ref = NULL;
 		GIT_UNUSED(ref);
 
 		if (packed_load(repo) < 0)
diff --git a/src/transports/winhttp.c b/src/transports/winhttp.c
index 64bfece..970fa53 100644
--- a/src/transports/winhttp.c
+++ b/src/transports/winhttp.c
@@ -88,7 +88,7 @@ static int apply_basic_credential(HINTERNET request, git_cred *cred)
 	git_cred_userpass_plaintext *c = (git_cred_userpass_plaintext *)cred;
 	git_buf buf = GIT_BUF_INIT, raw = GIT_BUF_INIT;
 	wchar_t *wide = NULL;
-	int error = -1, wide_len;
+	int error = -1, wide_len = 0;
 
 	git_buf_printf(&raw, "%s:%s", c->username, c->password);
 
diff --git a/tests-clar/clar_libgit2.c b/tests-clar/clar_libgit2.c
index 63efd59..698aa90 100644
--- a/tests-clar/clar_libgit2.c
+++ b/tests-clar/clar_libgit2.c
@@ -86,14 +86,18 @@ int cl_setenv(const char *name, const char *value)
 
 	git__utf8_to_16(name_utf16, GIT_WIN_PATH, name);
 
-	if (value != NULL)
+	if (value) {
 		git__utf8_to_16(value_utf16, GIT_WIN_PATH, value);
+		cl_assert(SetEnvironmentVariableW(name_utf16, value_utf16));
+	} else {
+		/* Windows XP returns 0 (failed) when passing NULL for lpValue when
+		 * lpName does not exist in the environment block. This behavior
+		 * seems to have changed in later versions. Don't check return value
+		 * of SetEnvironmentVariable when passing NULL for lpValue.
+		 */
+		SetEnvironmentVariableW(name_utf16, NULL);
+	}
 
-	/* Windows XP returns 0 (failed) when passing NULL for lpValue when lpName
-	 * does not exist in the environment block. This behavior seems to have changed
-	 * in later versions. Don't fail when SetEnvironmentVariable fails, if we passed
-	 * NULL for lpValue. */
-	cl_assert(SetEnvironmentVariableW(name_utf16, value ? value_utf16 : NULL) || !value);
 	return 0;
 }
 
diff --git a/tests-clar/core/env.c b/tests-clar/core/env.c
index fa48de1..2f5e91f 100644
--- a/tests-clar/core/env.c
+++ b/tests-clar/core/env.c
@@ -74,6 +74,8 @@ void test_core_env__0(void)
 	char **val;
 
 	memset(testfile, 0, sizeof(testfile));
+	cl_assert_equal_s("", testfile);
+
 	memcpy(testfile, "testfile", 8);
 	cl_assert_equal_s("testfile", testfile);