Merge pull request libgit2#5802 from lhchavez/git-warn-unused-result

Introduce GIT_WARN_UNUSED_RESULT

Author: ethomson

src/cc-compat.h

@@ -43,7 +43,15 @@
 #	define GIT_ALIGN(x,size) x
 #endif
 
-#define GIT_UNUSED(x) ((void)(x))
+#if defined(__GNUC__)
+# define GIT_UNUSED(x)                                                         \
+	do {                                                                   \
+		typeof(x) _unused __attribute__((unused));                     \
+		_unused = (x);                                                 \
+	} while (0)
+#else
+# define GIT_UNUSED(x) ((void)(x))
+#endif
 
 /* Define the printf format specifier to use for size_t output */
 #if defined(_MSC_VER) || defined(__MINGW32__)

src/common.h

@@ -30,6 +30,24 @@
 # define __has_builtin(x) 0
 #endif
 
+/**
+ * Declare that a function's return value must be used.
+ *
+ * Used mostly to guard against potential silent bugs at runtime. This is
+ * recommended to be added to functions that:
+ *
+ * - Allocate / reallocate memory. This prevents memory leaks or errors where
+ *   buffers are expected to have grown to a certain size, but could not be
+ *   resized.
+ * - Acquire locks. When a lock cannot be acquired, that will almost certainly
+ *   cause a data race / undefined behavior.
+ */
+#if defined(__GNUC__)
+# define GIT_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
+#else
+# define GIT_WARN_UNUSED_RESULT
+#endif
+
 #include <assert.h>
 #include <errno.h>
 #include <limits.h>

src/odb.c

@@ -573,7 +573,7 @@ int git_odb__add_default_backends(
 	git_odb *db, const char *objects_dir,
 	bool as_alternates, int alternate_depth)
 {
-	size_t i;
+	size_t i = 0;
 	struct stat st;
 	ino_t inode;
 	git_odb_backend *loose, *packed;
@@ -582,7 +582,7 @@ int git_odb__add_default_backends(
 	 * a cross-platform workaround for this */
 #ifdef GIT_WIN32
 	GIT_UNUSED(i);
-	GIT_UNUSED(st);
+	GIT_UNUSED(&st);
 
 	inode = 0;
 #else

src/path.c

@@ -1915,7 +1915,13 @@ GIT_INLINE(bool) should_validate_longpaths(git_repository *repo)
 }
 
 #else
-# define should_validate_longpaths(repo) (GIT_UNUSED(repo), false)
+
+GIT_INLINE(bool) should_validate_longpaths(git_repository *repo)
+{
+	GIT_UNUSED(repo);
+
+	return false;
+}
 #endif
 
 int git_path_validate_workdir(git_repository *repo, const char *path)

src/refdb_fs.c

@@ -122,7 +122,7 @@ static int packed_reload(refdb_fs_backend *backend)
 	 */
 	if (error <= 0) {
 		if (error == GIT_ENOTFOUND) {
-			git_sortedcache_clear(backend->refcache, true);
+			GIT_UNUSED(git_sortedcache_clear(backend->refcache, true));
 			git_error_clear();
 			error = 0;
 		}
@@ -131,7 +131,7 @@ static int packed_reload(refdb_fs_backend *backend)
 
 	/* At this point, refresh the packed refs from the loaded buffer. */
 
-	git_sortedcache_clear(backend->refcache, false);
+	GIT_UNUSED(git_sortedcache_clear(backend->refcache, false));
 
 	scan = (char *)packedrefs.ptr;
 	eof  = scan + packedrefs.size;
@@ -219,7 +219,7 @@ static int packed_reload(refdb_fs_backend *backend)
 parse_failed:
 	git_error_set(GIT_ERROR_REFERENCE, "corrupted packed references file");
 
-	git_sortedcache_clear(backend->refcache, false);
+	GIT_UNUSED(git_sortedcache_clear(backend->refcache, false));
 	git_sortedcache_wunlock(backend->refcache);
 	git_buf_dispose(&packedrefs);
 

src/sortedcache.h

@@ -58,7 +58,7 @@ typedef struct {
  *        may be NULL.  The cache makes it easy to load this and check
  *        if it has been modified since the last load and/or write.
  */
-int git_sortedcache_new(
+GIT_WARN_UNUSED_RESULT int git_sortedcache_new(
 	git_sortedcache **out,
 	size_t item_path_offset, /* use offsetof(struct, path-field) macro */
 	git_sortedcache_free_item_fn free_item,
@@ -71,7 +71,7 @@ int git_sortedcache_new(
  * - `copy_item` can be NULL to just use memcpy
  * - if `lock`, grabs read lock on `src` during copy and releases after
  */
-int git_sortedcache_copy(
+GIT_WARN_UNUSED_RESULT int git_sortedcache_copy(
 	git_sortedcache **out,
 	git_sortedcache *src,
 	bool lock,
@@ -100,7 +100,7 @@ const char *git_sortedcache_path(git_sortedcache *sc);
  */
 
 /* Lock sortedcache for write */
-int git_sortedcache_wlock(git_sortedcache *sc);
+GIT_WARN_UNUSED_RESULT int git_sortedcache_wlock(git_sortedcache *sc);
 
 /* Unlock sorted cache when done with write */
 void git_sortedcache_wunlock(git_sortedcache *sc);
@@ -120,7 +120,8 @@ void git_sortedcache_wunlock(git_sortedcache *sc);
  *
  * @return 0 if up-to-date, 1 if out-of-date, <0 on error
  */
-int git_sortedcache_lockandload(git_sortedcache *sc, git_buf *buf);
+GIT_WARN_UNUSED_RESULT int git_sortedcache_lockandload(
+	git_sortedcache *sc, git_buf *buf);
 
 /* Refresh file timestamp after write completes
  * You should already be holding the write lock when you call this.
@@ -132,12 +133,13 @@ void git_sortedcache_updated(git_sortedcache *sc);
  * If `wlock` is true, grabs write lock and releases when done, otherwise
  * you should already be holding a write lock when you call this.
  */
-int git_sortedcache_clear(git_sortedcache *sc, bool wlock);
+GIT_WARN_UNUSED_RESULT int git_sortedcache_clear(
+	git_sortedcache *sc, bool wlock);
 
 /* Find and/or insert item, returning pointer to item data.
  * You should already be holding the write lock when you call this.
  */
-int git_sortedcache_upsert(
+GIT_WARN_UNUSED_RESULT int git_sortedcache_upsert(
 	void **out, git_sortedcache *sc, const char *key);
 
 /* Removes entry at pos from cache
@@ -155,7 +157,7 @@ int git_sortedcache_remove(git_sortedcache *sc, size_t pos);
  */
 
 /* Lock sortedcache for read */
-int git_sortedcache_rlock(git_sortedcache *sc);
+GIT_WARN_UNUSED_RESULT int git_sortedcache_rlock(git_sortedcache *sc);
 
 /* Unlock sorted cache when done with read */
 void git_sortedcache_runlock(git_sortedcache *sc);

src/vector.h

@@ -26,11 +26,13 @@ typedef struct git_vector {
 
 #define GIT_VECTOR_INIT {0}
 
-int git_vector_init(git_vector *v, size_t initial_size, git_vector_cmp cmp);
+GIT_WARN_UNUSED_RESULT int git_vector_init(
+	git_vector *v, size_t initial_size, git_vector_cmp cmp);
 void git_vector_free(git_vector *v);
 void git_vector_free_deep(git_vector *v); /* free each entry and self */
 void git_vector_clear(git_vector *v);
-int git_vector_dup(git_vector *v, const git_vector *src, git_vector_cmp cmp);
+GIT_WARN_UNUSED_RESULT int git_vector_dup(
+	git_vector *v, const git_vector *src, git_vector_cmp cmp);
 void git_vector_swap(git_vector *a, git_vector *b);
 int git_vector_size_hint(git_vector *v, size_t size_hint);
 

src/win32/path_w32.h

@@ -8,7 +8,6 @@
 #define INCLUDE_win32_path_w32_h__
 
 #include "common.h"
-#include "vector.h"
 
 /**
  * Create a Win32 path (in UCS-2 format) from a UTF-8 string.  If the given

tests/core/sha1.c

@@ -52,7 +52,7 @@ void test_core_sha1__detect_collision_attack(void)
 	git_oid oid, expected;
 
 #ifdef GIT_SHA1_COLLISIONDETECT
-	GIT_UNUSED(expected);
+	GIT_UNUSED(&expected);
 	cl_git_fail(sha1_file(&oid, FIXTURE_DIR "/shattered-1.pdf"));
 	cl_assert_equal_s("SHA1 collision attack detected", git_error_last()->message);
 #else

tests/core/sortedcache.c

@@ -54,7 +54,7 @@ void test_core_sortedcache__name_only(void)
 	cl_assert_equal_i(
 		GIT_ENOTFOUND, git_sortedcache_lookup_index(&pos, sc, "abc"));
 
-	git_sortedcache_clear(sc, true);
+	cl_git_pass(git_sortedcache_clear(sc, true));
 
 	cl_assert_equal_sz(0, git_sortedcache_entrycount(sc));
 	cl_assert(git_sortedcache_entry(sc, 0) == NULL);
@@ -154,7 +154,7 @@ void test_core_sortedcache__in_memory(void)
 
 	cl_assert_equal_i(0, free_count);
 
-	git_sortedcache_clear(sc, true);
+	cl_git_pass(git_sortedcache_clear(sc, true));
 
 	cl_assert_equal_i(5, free_count);
 
@@ -247,7 +247,7 @@ static void sortedcache_test_reload(git_sortedcache *sc)
 
 	cl_assert(git_sortedcache_lockandload(sc, &buf) > 0);
 
-	git_sortedcache_clear(sc, false); /* clear once we already have lock */
+	cl_git_pass(git_sortedcache_clear(sc, false)); /* clear once we already have lock */
 
 	for (scan = buf.ptr; *scan; scan = after + 1) {
 		int val = strtol(scan, &after, 0);