Re: [PATCH] git-compat-util: convert skip_{prefix,suffix}{,_mem} to bool
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Dec 18, 2023, 16:23 UTC
- Message-ID
- <b7a56625-46d5-4d77-b4bf-5595a6fb2aef@gmail.com>
- In-Reply-To
- <2d30dc36-6091-4b47-846f-92d3f4a8b135@web.de>
Hi René
On 16/12/2023 10:47, René Scharfe wrote:
Show 23 quoted lines
> 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
Show 111 quoted lines
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
> 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 <stddef.h>
> #include <stdlib.h>
> #include <stdarg.h>
> +#include <stdbool.h>
> #include <string.h>
> #ifdef HAVE_STRINGS_H
> #include <strings.h> /* 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
>