Re: [PATCH v1 2/2] strbuf_getcwd() needs precompse_strbuf_if_needed()

From: Junio C Hamano <gitster@pobox.com>
Date: May 7, 2024, 17:22 UTC
Message-ID: <xmqqa5l1pmf9.fsf@gitster.g>
In-Reply-To: <20240507084431.19797-1-tboegi@web.de>

tboegi@web.de writes:

> +void precompse_strbuf_if_needed(struct strbuf *sb)
> +{
> +	char *buf_prec = (char *)precompose_string_if_needed(sb->buf);
> +	if (buf_prec != sb->buf) {

Cute. This matches with the !PRECOMPSE_UNICODE case in git-compat-util.h where we do

    static inline const char *precompose_string_if_needed(const char *in)
    {
            return in;
    }

to make it a no-op. I was wondering how you are avoiding an inevitable crash from trying to free an unfreeable piece of memory, but this should do just fine.

You'd want to fix the typo in the name of the new function, I presume? "precompse" -> "precompose"

Show 6 quoted lines

> +		size_t buf_prec_len = strlen(buf_prec);
> +		free(strbuf_detach(sb, NULL));
> +		strbuf_attach(sb, buf_prec, buf_prec_len, buf_prec_len + 1);
> +	}
> +
> +}

Show 10 quoted lines

> diff --git a/strbuf.c b/strbuf.c
> index 0d929e4e19..cefea6b75f 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -591,6 +591,7 @@ int strbuf_getcwd(struct strbuf *sb)
>  	for (;; guessed_len *= 2) {
>  		strbuf_grow(sb, guessed_len);
>  		if (getcwd(sb->buf, sb->alloc)) {
> +			precompse_strbuf_if_needed(sb);
>  			strbuf_setlen(sb, strlen(sb->buf));

The need for strbuf_setlen() stems from the use of getcwd() that may and will place a string that is much shorter than sb->alloc, so they logically belong together. It will make more sense to call the precompose _after_ arranging the members of strbuf in a consistent state with the call to strbuf_setlen().

>  			return 0;
>  		}
> --
> 2.41.0.394.ge43f4fd0bd