Re: [PATCH 1/3] Revert "make error()'s constant return value more visible"

[Jeff King <peff@peff.net>]

On Sun, May 04, 2014 at 01:12:53AM -0500, Felipe Contreras wrote:

> So it looks like gcc is smarter now, and in trying to fix a few warnings
> we generated hundreds more.
> 
> This reverts commit e208f9cc7574f5980faba498d0aa30b4defeb34f.

And now we've gone the other way, and re-enabled the initial warnings.
Can we come up with a solution that helps both cases?

I could not find a way to annotate a value as "maybe unused", but we can
hide it inside a function, like:

-- >8 --
Subject: inline error() function

The error() function does two things: it prints an error,
and it returns "-1" as a convenience to allow:

  return error("foo");

Commit e208f9c converted this to a macro to make the
constant return value more visible to the compiler. However,
recent versions of gcc complain when error is used in a void
context, as the constant "-1" ends up unused.

Instead, let's convert error() to a static inline, which
should accomplish the same thing without the extra warnings
(because gcc will not warn about unused return values unless
warn_unused_result is specified for the particular
function).

Signed-off-by: Jeff King <peff@peff.net>
---
 git-compat-util.h | 20 ++++++++++++--------
 usage.c           |  8 +-------
 2 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index f6d3a46..3aef0d3 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -310,7 +310,6 @@ extern NORETURN void usage(const char *err);
 extern NORETURN void usagef(const char *err, ...) __attribute__((format (printf, 1, 2)));
 extern NORETURN void die(const char *err, ...) __attribute__((format (printf, 1, 2)));
 extern NORETURN void die_errno(const char *err, ...) __attribute__((format (printf, 1, 2)));
-extern int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
 extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)));
 
 #ifndef NO_OPENSSL
@@ -325,14 +324,19 @@ extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)))
 
 /*
  * Let callers be aware of the constant return value; this can help
- * gcc with -Wuninitialized analysis. We restrict this trick to gcc, though,
- * because some compilers may not support variadic macros. Since we're only
- * trying to help gcc, anyway, it's OK; other compilers will fall back to
- * using the function as usual.
+ * gcc with -Wuninitialized analysis.
  */
-#if defined(__GNUC__) && ! defined(__clang__)
-#define error(...) (error(__VA_ARGS__), -1)
-#endif
+__attribute__((format (printf, 1, 0)))
+extern void error_impl(const char *err, va_list params);
+__attribute__((format (printf, 1, 2)))
+static inline int error(const char *err, ...)
+{
+    va_list params;
+    va_start(params, err);
+    error_impl(err, params);
+    va_end(params);
+    return -1;
+}
 
 extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params));
 extern void set_error_routine(void (*routine)(const char *err, va_list params));
diff --git a/usage.c b/usage.c
index ed14645..5456e4b 100644
--- a/usage.c
+++ b/usage.c
@@ -138,15 +138,9 @@ void NORETURN die_errno(const char *fmt, ...)
 	va_end(params);
 }
 
-#undef error
-int error(const char *err, ...)
+void error_impl(const char *err, va_list params)
 {
-	va_list params;
-
-	va_start(params, err);
 	error_routine(err, params);
-	va_end(params);
-	return -1;
 }
 
 void warning(const char *warn, ...)
-- 
2.0.0.rc1.436.g03cb729