From 9b240347543f240dbf7e541173ac45c34155ca1b Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 19 Aug 2022 06:08:27 -0400 Subject: git-compat-util: add UNUSED macro In preparation for compiling with -Wunused-parameter, we'd like to be able to annotate some function parameters as false positives (e.g., parameters which must exist to conform to a callback interface). Ideally our annotation will: - be portable, turning into nothing on platforms which don't support it - be easy to read, without looking too syntactically odd or taking attention away from the rest of the parameters - help us notice when a parameter marked as unused is actually used, which keeps our annotations accurate. In theory a compiler could tell us this easily, but gcc has no such warning. Clang has -Wused-but-marked-unused, but it triggers false positives with our MAYBE_UNUSED annotation (e.g., for commit-slab functions) This patch introduces an UNUSED() macro which takes the parameter name as an argument. That lets us tweak the name in such a way that we'll notice if somebody tries to use it. It looks like this in use: int some_ref_cb(const char *refname, const struct object_id *UNUSED(oid), int UNUSED(flags), void *UNUSED(data)) { printf("got refname %s", refname); return 0; } Because the unused parameter names are rewritten behind the scenes to UNUSED_oid, etc, adding code like: printf("oid is %s", oid_to_hex(oid)); will fail compilation with "oid undeclared". Sadly, the "did you mean" feature of modern compilers is not generally smart enough to suggest the "unused" name. If we used a very short prefix like U_oid, that does convince gcc to say "did you mean", but since the "U_" in the suggestion isn't much of a hint, it doesn't really help. In practice, a look at the function definition usually makes the problem pretty obvious. Note that we have to put the definition of UNUSED early in git-compat-util.h, because it will eventually be used for some compat functions themselves (both directly here and in mingw.h). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- git-compat-util.h | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'git-compat-util.h') diff --git a/git-compat-util.h b/git-compat-util.h index 36a25ae252..c6669db07d 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -189,6 +189,12 @@ struct strbuf; #define _NETBSD_SOURCE 1 #define _SGI_SOURCE 1 +#if defined(__GNUC__) +#define UNUSED(var) UNUSED_##var __attribute__((unused)) +#else +#define UNUSED(var) UNUSED_##var +#endif + #if defined(WIN32) && !defined(__CYGWIN__) /* Both MinGW and MSVC */ # if !defined(_WIN32_WINNT) # define _WIN32_WINNT 0x0600 -- cgit v1.2.3 From 783a86c1427637d71fb2710291e677360ab5dc09 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 19 Aug 2022 06:08:44 -0400 Subject: config: mark unused callback parameters The callback passed to git_config() must conform to a particular interface. But most callbacks don't actually look at the extra "void *data" parameter. Let's mark the unused parameters to make -Wunused-parameter happy. Note there's one unusual case here in get_remote_default() where we actually ignore the "value" parameter. That's because it's only checking whether the option is found at all, and not parsing its value. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- git-compat-util.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'git-compat-util.h') diff --git a/git-compat-util.h b/git-compat-util.h index c6669db07d..12239fedf7 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -403,7 +403,9 @@ typedef uintmax_t timestamp_t; #endif #ifndef platform_core_config -static inline int noop_core_config(const char *var, const char *value, void *cb) +static inline int noop_core_config(const char *UNUSED(var), + const char *UNUSED(value), + void *UNUSED(cb)) { return 0; } -- cgit v1.2.3 From 776515ef8b381d49caeccfe2e8da98cb666e257a Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 19 Aug 2022 06:08:54 -0400 Subject: is_path_owned_by_current_uid(): mark "report" parameter as unused In the non-Windows version of this function, we never have any errors to report, and thus the "report" parameter is unused. But we can't drop it, because we have to maintain function call compatibility with the version in compat/mingw.h, which does use this parameter. Note that there's an extra level of indirection here; the common function is actually is_path_owned_by_current_user, which is a macro pointing to "by_current_uid" or "by_current_sid", depending on the platform. So an alternative here is to eat the unused parameter in the macro, since -Wunused-parameter doesn't complain about macros. But I think the UNUSED() annotation is less obfuscated for somebody reading the code later. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- git-compat-util.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'git-compat-util.h') diff --git a/git-compat-util.h b/git-compat-util.h index 12239fedf7..a9690126bb 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -498,7 +498,8 @@ static inline void extract_id_from_env(const char *env, uid_t *id) } } -static inline int is_path_owned_by_current_uid(const char *path, struct strbuf *report) +static inline int is_path_owned_by_current_uid(const char *path, + struct strbuf *UNUSED(report)) { struct stat st; uid_t euid; -- cgit v1.2.3