Re: [PATCH 1/3] fixup: use xstrfmt instead of fixed-size buf + sprintf + xstrdup
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 12, 2015, 17:45 UTC
- Message-ID
- <xmqqk2qsm14x.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <1444580188-18773-1-git-send-email-Matthieu.Moy@imag.fr>
Matthieu Moy <Matthieu.Moy@imag.fr> writes:
Show 7 quoted lines
> The char buf[40] is safe (at least while the strings are not > translated), but I'd rather avoid magic numbers like this 40 in the > code, and use a construct that does not have this size limitation. > Especially if it makes the code shorter. > > Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr> > ---
The construct being fixed with this change looks very similar to Peff's a5e03bf5 (ref-filter: drop sprintf and strcpy calls, 2015-09-24) on jk/war-on-sprintf topic, but the new code since that commit cleaned up.
I'd expect that this will be rolled into Karthik's series in the next reroll?
Looking good. Thanks.
Show 48 quoted lines
> ref-filter.c | 20 ++++++++------------
> 1 file changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/ref-filter.c b/ref-filter.c
> index 6044eb0..7932c21 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1116,7 +1116,6 @@ static void populate_value(struct ref_array_item *ref)
> strcmp(formatp, "trackshort") &&
> (starts_with(name, "upstream") ||
> starts_with(name, "push"))) {
> - char buf[40];
> unsigned int nobracket = 0;
>
> if (!strcmp(valp, ",nobracket"))
> @@ -1135,24 +1134,21 @@ static void populate_value(struct ref_array_item *ref)
> v->s = "";
> else if (!num_ours) {
> if (nobracket)
> - sprintf(buf, "behind %d", num_theirs);
> + v->s = xstrfmt("behind %d", num_theirs);
> else
> - sprintf(buf, "[behind %d]", num_theirs);
> - v->s = xstrdup(buf);
> + v->s = xstrfmt("[behind %d]", num_theirs);
> } else if (!num_theirs) {
> if (nobracket)
> - sprintf(buf, "ahead %d", num_ours);
> + v->s = xstrfmt("ahead %d", num_ours);
> else
> - sprintf(buf, "[ahead %d]", num_ours);
> - v->s = xstrdup(buf);
> + v->s = xstrfmt("[ahead %d]", num_ours);
> } else {
> if (nobracket)
> - sprintf(buf, "ahead %d, behind %d",
> - num_ours, num_theirs);
> + v->s = xstrfmt("ahead %d, behind %d",
> + num_ours, num_theirs);
> else
> - sprintf(buf, "[ahead %d, behind %d]",
> - num_ours, num_theirs);
> - v->s = xstrdup(buf);
> + v->s = xstrfmt("[ahead %d, behind %d]",
> + num_ours, num_theirs);
> }
> continue;
> } else if (!strcmp(formatp, "trackshort") &&