From: Phillip Wood Date: Mon, 18 Dec 2023 16:23:19 GMT Subject: Re: [PATCH] git-compat-util: convert skip_{prefix,suffix}{,_mem} to bool Message-ID: In-Reply-To: <2d30dc36-6091-4b47-846f-92d3f4a8b135@web.de> Hi René On 16/12/2023 10:47, René Scharfe wrote: > Use the data type bool and its values true and false to document the > binary return value of skip_prefix() and friends more explicitly. > > This first use of stdbool.h, introduced with C99, is meant to check > whether there are platforms that claim support for C99, as tested by > 7bc341e21b (git-compat-util: add a test balloon for C99 support, > 2021-12-01), but still lack that header for some reason. > > A fallback based on a wider type, e.g. int, would have to deal with > comparisons somehow to emulate that any non-zero value is true: > > bool b1 = 1; > bool b2 = 2; > if (b1 == b2) puts("This is true."); > > int i1 = 1; > int i2 = 2; > if (i1 == i2) puts("Not printed."); > #define BOOLEQ(a, b) (!(a) == !(b)) > if (BOOLEQ(i1, i2)) puts("This is true."); > > So we'd be better off using bool everywhere without a fallback, if > possible. That's why this patch doesn't include any. Thanks for the comprehensive commit message, I agree that we'd be better off avoiding adding a fallback. The patch looks good, I did wonder if we really need to covert all of these functions for a test-balloon but the patch is still pretty small overall. Best Wishes Phillip > Signed-off-by: René Scharfe > --- > git-compat-util.h | 42 ++++++++++++++++++++++-------------------- > 1 file changed, 22 insertions(+), 20 deletions(-) > > diff --git a/git-compat-util.h b/git-compat-util.h > index 3e7a59b5ff..603c97e3b3 100644 > --- a/git-compat-util.h > +++ b/git-compat-util.h > @@ -225,6 +225,7 @@ struct strbuf; > #include > #include > #include > +#include > #include > #ifdef HAVE_STRINGS_H > #include /* for strcasecmp() */ > @@ -684,11 +685,11 @@ report_fn get_warn_routine(void); > void set_die_is_recursing_routine(int (*routine)(void)); > > /* > - * If the string "str" begins with the string found in "prefix", return 1. > + * If the string "str" begins with the string found in "prefix", return true. > * The "out" parameter is set to "str + strlen(prefix)" (i.e., to the point in > * the string right after the prefix). > * > - * Otherwise, return 0 and leave "out" untouched. > + * Otherwise, return false and leave "out" untouched. > * > * Examples: > * > @@ -699,57 +700,58 @@ void set_die_is_recursing_routine(int (*routine)(void)); > * [skip prefix if present, otherwise use whole string] > * skip_prefix(name, "refs/heads/", &name); > */ > -static inline int skip_prefix(const char *str, const char *prefix, > - const char **out) > +static inline bool skip_prefix(const char *str, const char *prefix, > + const char **out) > { > do { > if (!*prefix) { > *out = str; > - return 1; > + return true; > } > } while (*str++ == *prefix++); > - return 0; > + return false; > } > > /* > * Like skip_prefix, but promises never to read past "len" bytes of the input > * buffer, and returns the remaining number of bytes in "out" via "outlen". > */ > -static inline int skip_prefix_mem(const char *buf, size_t len, > - const char *prefix, > - const char **out, size_t *outlen) > +static inline bool skip_prefix_mem(const char *buf, size_t len, > + const char *prefix, > + const char **out, size_t *outlen) > { > size_t prefix_len = strlen(prefix); > if (prefix_len <= len && !memcmp(buf, prefix, prefix_len)) { > *out = buf + prefix_len; > *outlen = len - prefix_len; > - return 1; > + return true; > } > - return 0; > + return false; > } > > /* > - * If buf ends with suffix, return 1 and subtract the length of the suffix > - * from *len. Otherwise, return 0 and leave *len untouched. > + * If buf ends with suffix, return true and subtract the length of the suffix > + * from *len. Otherwise, return false and leave *len untouched. > */ > -static inline int strip_suffix_mem(const char *buf, size_t *len, > - const char *suffix) > +static inline bool strip_suffix_mem(const char *buf, size_t *len, > + const char *suffix) > { > size_t suflen = strlen(suffix); > if (*len < suflen || memcmp(buf + (*len - suflen), suffix, suflen)) > - return 0; > + return false; > *len -= suflen; > - return 1; > + return true; > } > > /* > - * If str ends with suffix, return 1 and set *len to the size of the string > - * without the suffix. Otherwise, return 0 and set *len to the size of the > + * If str ends with suffix, return true and set *len to the size of the string > + * without the suffix. Otherwise, return false and set *len to the size of the > * string. > * > * Note that we do _not_ NUL-terminate str to the new length. > */ > -static inline int strip_suffix(const char *str, const char *suffix, size_t *len) > +static inline bool strip_suffix(const char *str, const char *suffix, > + size_t *len) > { > *len = strlen(str); > return strip_suffix_mem(str, len, suffix); > -- > 2.43.0 >