Commit f9b55bcb5f9fd8c41c74d4be58ebaad13aa9b7f3

Scott J. Goldman 2013-01-05T18:20:42

git_mwindow_file_deregister() shouldn't return errors As a function that appears to only be called on error paths, I don't think it makes sense for it to return an error, or clobber the global giterr. Note that no existing callsites actually check the return code. In my own application, there are errors where the real error ends up being hidden, as git_mwindow_file_deregister() clobbers the global giterr. I'm not sure this error is even relevant?

diff --git a/src/mwindow.c b/src/mwindow.c
index ee693e4..64eba01 100644
--- a/src/mwindow.c
+++ b/src/mwindow.c
@@ -293,28 +293,23 @@ int git_mwindow_file_register(git_mwindow_file *mwf)
 	return ret;
 }
 
-int git_mwindow_file_deregister(git_mwindow_file *mwf)
+void git_mwindow_file_deregister(git_mwindow_file *mwf)
 {
 	git_mwindow_ctl *ctl = &mem_ctl;
 	git_mwindow_file *cur;
 	unsigned int i;
 
-	if (git_mutex_lock(&git__mwindow_mutex)) {
-		giterr_set(GITERR_THREAD, "unable to lock mwindow mutex");
-		return -1;
-	}
+	if (git_mutex_lock(&git__mwindow_mutex))
+		return;
 
 	git_vector_foreach(&ctl->windowfiles, i, cur) {
 		if (cur == mwf) {
 			git_vector_remove(&ctl->windowfiles, i);
 			git_mutex_unlock(&git__mwindow_mutex);
-			return 0;
+			return;
 		}
 	}
 	git_mutex_unlock(&git__mwindow_mutex);
-
-	giterr_set(GITERR_ODB, "Failed to find the memory window file to deregister");
-	return -1;
 }
 
 void git_mwindow_close(git_mwindow **window)
diff --git a/src/mwindow.h b/src/mwindow.h
index c5aeaf7..dc58a86 100644
--- a/src/mwindow.h
+++ b/src/mwindow.h
@@ -39,7 +39,7 @@ int git_mwindow_contains(git_mwindow *win, git_off_t offset);
 void git_mwindow_free_all(git_mwindow_file *mwf);
 unsigned char *git_mwindow_open(git_mwindow_file *mwf, git_mwindow **cursor, git_off_t offset, size_t extra, unsigned int *left);
 int git_mwindow_file_register(git_mwindow_file *mwf);
-int git_mwindow_file_deregister(git_mwindow_file *mwf);
+void git_mwindow_file_deregister(git_mwindow_file *mwf);
 void git_mwindow_close(git_mwindow **w_cursor);
 
 #endif