Re: [PATCH 15/67] convert trivial sprintf / strcpy calls to xsnprintf

On Tue, Sep 15, 2015 at 01:38:42PM -0700, Stefan Beller wrote: > Some off topic thoughts: > > Having an "assert" behavior is not a good user experience though > and should be fixed. To fix it we need stack traces. And the git > version. And telling the user to send it to the mailing list. Yes,

Re: [PATCH 15/67] convert trivial sprintf / strcpy calls to xsnprintf

On Tue, Sep 15, 2015 at 11:19:21PM -0400, Eric Sunshine wrote: > > strcpy(hexbuf[stage], sha1_to_hex(ce->sha1)); > > - sprintf(ownbuf[stage], "%o", ce->ce_mode); > > + xsnprintf(ownbuf[stage], sizeof(ownbuf[stage]), "%o", > > ce->ce_mode); > >

Re: [PATCH 15/67] convert trivial sprintf / strcpy calls to xsnprintf

Jeff King writes: > I think a core file is even more useful than a backtrace. Having BUG() > call abort() would be even more useful, I think. Sounds like a plan ;-) Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to

Re: [PATCH 15/67] convert trivial sprintf / strcpy calls to xsnprintf

On Wed, Sep 16, 2015 at 12:07 PM, Junio C Hamano wrote: > Jeff King writes: > >> On Wed, Sep 16, 2015 at 11:24:27AM -0700, Junio C Hamano wrote: >> >>> Jeff King writes: >>> >>> > On Tue, Sep 15, 2015 at 11:19:21PM -0400, Eric Sunshine wrote: >>>

Re: [PATCH 15/67] convert trivial sprintf / strcpy calls to xsnprintf

Jeff King writes: > On Wed, Sep 16, 2015 at 11:24:27AM -0700, Junio C Hamano wrote: > >> Jeff King writes: >> >> > On Tue, Sep 15, 2015 at 11:19:21PM -0400, Eric Sunshine wrote: >> > >> >> > strcpy(hexbuf[stage], sha1_to_hex(ce->sha1)); >> >> > -

Re: [PATCH 15/67] convert trivial sprintf / strcpy calls to xsnprintf

Jeff King writes: > On Tue, Sep 15, 2015 at 11:19:21PM -0400, Eric Sunshine wrote: > >> > strcpy(hexbuf[stage], sha1_to_hex(ce->sha1)); >> > - sprintf(ownbuf[stage], "%o", ce->ce_mode); >> > + xsnprintf(ownbuf[stage],

Re: [PATCH 15/67] convert trivial sprintf / strcpy calls to xsnprintf

On Wed, Sep 16, 2015 at 12:07:30PM -0700, Junio C Hamano wrote: > > Correct. The original is sane and gcc does the right thing. The question > > is whether some compiler would complain that "stage" is not a constant > > in the sizeof() expression. I don't know if any compiler would do so, > > but

Re: [PATCH 15/67] convert trivial sprintf / strcpy calls to xsnprintf

On Wed, Sep 16, 2015 at 12:19:10PM -0700, Stefan Beller wrote: > > That will make future readers wonder "Is this a typo, and if it is, > > which index is a mistake I can fix?" and may lead to an unnecessary > > confusion. I do not want to see a correctly written > > > >

Re: [PATCH 15/67] convert trivial sprintf / strcpy calls to xsnprintf

On Wed, Sep 16, 2015 at 11:24:27AM -0700, Junio C Hamano wrote: > Jeff King writes: > > > On Tue, Sep 15, 2015 at 11:19:21PM -0400, Eric Sunshine wrote: > > > >> > strcpy(hexbuf[stage], sha1_to_hex(ce->sha1)); > >> > - sprintf(ownbuf[stage], "%o",

Re: [PATCH 15/67] convert trivial sprintf / strcpy calls to xsnprintf

On Tue, Sep 15, 2015 at 11:36 AM, Jeff King wrote: > We sometimes sprintf into static buffers when we know that > the size of the buffer is large enough to fit the input > (either because it's a constant, or because it's numeric > input that is bounded in size). Likewise with

Re: [PATCH 15/67] convert trivial sprintf / strcpy calls to xsnprintf

Ramsay Jones writes: > How about using strlcpy() instead? Thus: > > - strcpy(header.name, "pax_global_header"); > + strlcpy(header.name, "pax_global_header", sizeof(header.name)); > > Ditto for other similar (strcpy->xsnprintf) hunks below. Please do not

Re: [PATCH 15/67] convert trivial sprintf / strcpy calls to xsnprintf

On 15/09/15 16:36, Jeff King wrote: > We sometimes sprintf into static buffers when we know that > the size of the buffer is large enough to fit the input > (either because it's a constant, or because it's numeric > input that is bounded in size). Likewise with strcpy of > constant strings. > >

Re: [PATCH 15/67] convert trivial sprintf / strcpy calls to xsnprintf

On Tue, Sep 15, 2015 at 07:32:29PM +0100, Ramsay Jones wrote: > > diff --git a/archive-tar.c b/archive-tar.c > > index b6b30bb..d543f93 100644 > > --- a/archive-tar.c > > +++ b/archive-tar.c > > @@ -301,7 +301,7 @@ static int write_global_extended_header(struct > > archiver_args *args) > >

Re: [PATCH 15/67] convert trivial sprintf / strcpy calls to xsnprintf

On 15/09/15 19:42, Jeff King wrote: > On Tue, Sep 15, 2015 at 07:32:29PM +0100, Ramsay Jones wrote: > >>> diff --git a/archive-tar.c b/archive-tar.c >>> index b6b30bb..d543f93 100644 >>> --- a/archive-tar.c >>> +++ b/archive-tar.c >>> @@ -301,7 +301,7 @@ static int

[PATCH 15/67] convert trivial sprintf / strcpy calls to xsnprintf

We sometimes sprintf into static buffers when we know that the size of the buffer is large enough to fit the input (either because it's a constant, or because it's numeric input that is bounded in size). Likewise with strcpy of constant strings. However, these sites make it hard to audit sprintf

Re: [PATCH 15/67] convert trivial sprintf / strcpy calls to xsnprintf

On Tue, Sep 15, 2015 at 11:42 AM, Jeff King wrote: > That misses the "assert" behavior of xsnprintf. When I saw the first patches of this series, I like them. Some off topic thoughts: Having an "assert" behavior is not a good user experience though and should be fixed. To fix it