Re: [PATCHv3 2/2] builtin/commit.c: switch to xstrfmt(), instead of snprintf,

From: Elia Pinto <gitter.spiros@gmail.com>
Date: Jan 15, 2017, 08:13 UTC
Message-ID: <CA+EOSB=4-TKpi6mr-yVbwRsFrVzE=vo4Y9Qm3VMm7pn=UB1_hQ@mail.gmail.com>
In-Reply-To: <xmqqr345cacz.fsf@gitster.mtv.corp.google.com>

2017-01-15 3:42 GMT+01:00 Junio C Hamano <gitster@pobox.com>:

Show 9 quoted lines

> Elia Pinto <gitter.spiros@gmail.com> writes:
>
>> Ok. I agree. But  is it strictly necessary to resend for this ?
>
> FWIW, the attacched is what I queued locally, after complaining
> "both have the same title?  They need to be explained better."
>
> In any case, I sense that 2/2 will be redone using strbuf, from the
> looks of what is discussed in a subthread nearby?

Yes, it seems to me correct, I resend the patch 2/2 based on the review

thank you all

Show 40 quoted lines

>
> Thanks.
>
> commit 8d7aa4ba6a00b3ff69261e88b4842c0df5662125
> Author: Elia Pinto <gitter.spiros@gmail.com>
> Date:   Fri Jan 13 17:58:00 2017 +0000
>
>     builtin/commit.c: remove the PATH_MAX limitation via dynamic allocation
>
>     Remove the PATH_MAX limitation from the environment setting that
>     points to a filename by switching to dynamic allocation.
>
>     As a side effect of this change, we also reduce the snprintf()
>     calls, that may silently truncate results if the programmer is not
>     careful.
>
>     Helped-by: Junio C Hamano <gitster@pobox.com>
>     Helped-by: Jeff King <peff@peff.net>
>     Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
>     Signed-off-by: Junio C Hamano <gitster@pobox.com>
>
> commit 2a7e328877982557d921a398af9442093290c613
> Author: Elia Pinto <gitter.spiros@gmail.com>
> Date:   Fri Jan 13 17:58:01 2017 +0000
>
>     builtin/commit.c: switch to xstrfmt(), instead of snprintf()
>
>     Switch to dynamic allocation with xstrfmt(), so we can avoid dealing
>     with magic numbers in the code and reduce the cognitive burden from
>     the programmers.  The original code is correct, but programmers no
>     longer have to count bytes needed for static allocation to know that.
>
>     As a side effect of this change, we also reduce the snprintf()
>     calls, that may silently truncate results if the programmer is not
>     careful.
>
>     Helped-by: Junio C Hamano <gitster@pobox.com>
>     Helped-by: Jeff King <peff@peff.net>
>     Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
>     Signed-off-by: Junio C Hamano <gitster@pobox.com>