"brian m. carlson" [off-list ref] writes:

On 2020-08-06 at 23:47:34, Junio C Hamano wrote:

quoted

I am wondering if it is a good idea to use something like

    static inline void precompose_argv(int argc, const char **argv)
    {
	; /* nothing */
    }

instead.  As long as the compiler is reasonable enough, this should
not result in any code change in the result, except that it would
still catch wrong arguments, even if these two parameters are unused
and optimized out.

Yes, this seems like a prudent approach.  I believe it's widely used by
the Linux kernel, so presumably compilers are capable enough to optimize
it out.  As you noted, it provides type checking for all platforms,
which is nice.

So I hope the following (untested and not signed off yet) may lead
us in the right direction?

-- >8 --
Subject: compat-util: type-check parameters of mocked functions

When there is no need to run a specific function on certain platforms,
we often #define an empty function to swallow its parameters and
make it into a no-op, e.g.

    #define precompose_argv(c,v) /* no-op */

While this guarantees that no unneeded code is generated, it also
discards type and other checks on these parameters, e.g. a new code
written with the argv-array API (diff_args is of type "struct
argv_array" that has .argc and .argv members):

    precompose_argv(diff_args.argc, diff_args.argv);

must be updated to use "struct strvec diff_args" with .nr and .v
members, like so:

    precompose_argv(diff_args.nr, diff_args.v);

after the argv-array API has been updated to the strvec API.
However, the "no oop" C preprocessor macro is too aggressive to
discard what is unused, and did not catch such a call that was left
unconverted.

Using a "static inline" function whose body is a no-op should still
result in the same binary with decent compilers yet catch such a
reference to a missing field or passing a value of a wrong type.

While at it, I notice that precompute_str() has never been used
anywhere in the code, since it was introduced at 76759c7d (git on
Mac OS and precomposed unicode, 2012-07-08).  Instead of turning it
into a static inline, just remove it.

---
 git-compat-util.h | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index 5637114b8d..7a0fb7a045 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h

@@ -252,8 +252,10 @@ typedef unsigned long uintptr_t;
 #ifdef PRECOMPOSE_UNICODE
 #include "compat/precompose_utf8.h"
 #else
-#define precompose_str(in,i_nfd2nfc)
-#define precompose_argv(c,v)
+static inline void precompose_argv(int argc, const char **argv)
+{
+	; /* nothing */
+}
 #define probe_utf8_pathname_composition()
 #endif
 

@@ -270,7 +272,9 @@ struct itimerval {
 #endif
 
 #ifdef NO_SETITIMER
-#define setitimer(which,value,ovalue)
+static inline int setitimer(int which, const struct itimerval *value, struct itimerval *newvalue) {
+	; /* nothing */
+}
 #endif
 
 #ifndef NO_LIBGEN_H

@@ -1231,8 +1235,14 @@ int warn_on_fopen_errors(const char *path);
 #endif
 
 #ifndef _POSIX_THREAD_SAFE_FUNCTIONS
-#define flockfile(fh)
-#define funlockfile(fh)
+static inline void flockfile(FILE *fh)
+{
+	; /* nothing */
+}
+static inline void funlockfile(FILE *fh)
+{
+	; /* nothing */
+}
 #define getc_unlocked(fh) getc(fh)
 #endif