From 2121a76d71e6742fe9627289b45717663bcef832 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 6 Dec 2024 11:27:16 +0100 Subject: git-compat-util: introduce macros to disable "-Wsign-compare" warnings When compiling with DEVELOPER=YesPlease, we explicitly disable the "-Wsign-compare" warning. This is mostly because our code base is full of cases where we don't bother at all whether something should be signed or unsigned, and enabling the warning would thus cause tons of warnings to pop up. Unfortunately, disabling this warning also masks real issues. There have been multiple CVEs in the Git project that would have been flagged by this warning (e.g. CVE-2022-39260, CVE-2022-41903 and several fixes in the vicinity of these CVEs). Furthermore, the final audit report by X41 D-Sec, who are the ones who have discovered some of the CVEs, hinted that it might be a good idea to become more strict in this context. Now simply enabling the warning globally does not fly due to the stated reason above that we simply have too many sites where we use the wrong integer types. Instead, introduce a new set of macros that allow us to mark a file as being free of warnings with "-Wsign-compare". The mechanism is similar to what we do with `USE_THE_REPOSITORY_VARIABLE`: every file that is not marked with `DISABLE_SIGN_COMPARE_WARNINGS` will be compiled with those warnings enabled. These new markings will be wired up in the subsequent commits. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- git-compat-util.h | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'git-compat-util.h') diff --git a/git-compat-util.h b/git-compat-util.h index a06d4f3809..e283c46c6f 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -44,6 +44,16 @@ struct strbuf; #define GIT_GNUC_PREREQ(maj, min) 0 #endif +#if defined(__GNUC__) || defined(__clang__) +# define PRAGMA(pragma) _Pragma(#pragma) +# define DISABLE_WARNING(warning) PRAGMA(GCC diagnostic ignored #warning) +#else +# define DISABLE_WARNING(warning) +#endif + +#ifdef DISABLE_SIGN_COMPARE_WARNINGS +DISABLE_WARNING(-Wsign-compare) +#endif #ifndef FLEX_ARRAY /* -- cgit v1.3 From 0148fd836a9b1954833471f61b4d2e058797af55 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 16 Jan 2025 13:35:50 -0800 Subject: usage: add show_usage_if_asked() Some commands call usage() when they are asked to give the help message with "git cmd -h", but this has the same problem as we fixed with callers of usage_with_options() for the same purpose. Introduce a helper function that captures the common pattern if (argc == 2 && !strcmp(argv[1], "-h")) usage(usage); and replaces it with show_usage_if_asked(argc, argv, usage); to help correct these code paths. Note that this helper function still exits with status 129, and t0012 insists on it. After converting all the mistaken callers of usage_with_options() to call this new helper, we may want to address it---the end user is asking us to give the help text, and we are doing exactly as asked, so there is no reason to exit with non-zero status. Helped-by: Jeff King Signed-off-by: Junio C Hamano --- git-compat-util.h | 2 ++ usage.c | 27 ++++++++++++++++++++++++--- 2 files changed, 26 insertions(+), 3 deletions(-) (limited to 'git-compat-util.h') diff --git a/git-compat-util.h b/git-compat-util.h index e283c46c6f..d43dd248c4 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -701,6 +701,8 @@ int error_errno(const char *err, ...) __attribute__((format (printf, 1, 2))); void warning(const char *err, ...) __attribute__((format (printf, 1, 2))); void warning_errno(const char *err, ...) __attribute__((format (printf, 1, 2))); +void show_usage_if_asked(int ac, const char **av, const char *err); + #ifndef NO_OPENSSL #ifdef APPLE_COMMON_CRYPTO #include "compat/apple-common-crypto.h" diff --git a/usage.c b/usage.c index 47709006c1..38b46bbbfe 100644 --- a/usage.c +++ b/usage.c @@ -8,7 +8,7 @@ #include "gettext.h" #include "trace2.h" -static void vreportf(const char *prefix, const char *err, va_list params) +static void vfreportf(FILE *f, const char *prefix, const char *err, va_list params) { char msg[4096]; char *p, *pend = msg + sizeof(msg); @@ -32,8 +32,13 @@ static void vreportf(const char *prefix, const char *err, va_list params) } *(p++) = '\n'; /* we no longer need a NUL */ - fflush(stderr); - write_in_full(2, msg, p - msg); + fflush(f); + write_in_full(fileno(f), msg, p - msg); +} + +static void vreportf(const char *prefix, const char *err, va_list params) +{ + vfreportf(stderr, prefix, err, params); } static NORETURN void usage_builtin(const char *err, va_list params) @@ -173,6 +178,22 @@ void NORETURN usage(const char *err) usagef("%s", err); } +static void show_usage_if_asked_helper(const char *err, ...) +{ + va_list params; + + va_start(params, err); + vfreportf(stdout, _("usage: "), err, params); + va_end(params); + exit(129); +} + +void show_usage_if_asked(int ac, const char **av, const char *err) +{ + if (ac == 2 && !strcmp(av[1], "-h")) + show_usage_if_asked_helper(err); +} + void NORETURN die(const char *err, ...) { va_list params; -- cgit v1.3