Re: [PATCH] git-compat-util: convert skip_{prefix,suffix}{,_mem} to bool

[Phillip Wood <phillip.wood123@gmail.com>]

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 <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
>