Re: [PATCH 1/3] fixup: use xstrfmt instead of fixed-size buf + sprintf + xstrdup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Matthieu Moy <Matthieu.Moy@xxxxxxx> writes:

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

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.

>  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") &&
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]