On Thu, 2007-09-06 at 20:08 +0200, Pierre Habouzit wrote:
quoted text
> On Thu, Sep 06, 2007 at 05:59:29PM +0000, Kristian Høgsberg wrote:
> > On Thu, 2007-09-06 at 13:20 +0200, Pierre Habouzit wrote:
> > > +	strbuf_grow(sb, len);
> > > +	strbuf_addf(sb, "%u %s=", len, keyword);
> > > +	strbuf_add(sb, value, valuelen);
> > > +	strbuf_addch(sb, '\n');
> > >  }
> > 
> > This entire function can be collapsed to just:
> > 
> > 	strbuf_addf(sb, "%u %s=%.*s\n", len, keyword, valuelen, value);
> 
>   yes, but it's less efficient, because %.*s says that sprintf must copy
> at most valuelen bytes from value, but it still has to stop if it finds
> a __PLACEHOLDER__0_ before. And the strbuf_grow has sense because the extend policy at
> snprintf time is optimistic: we try to write, and if it didn't fit, we
> try again. So there is a huge benefit if we have a clue of the final
> size.

Yeah, my strbuf_addf() suggestion doesn't work, since the format seems
to indicate the embedded NULs indeed is allowed (the %u is the length of
the value), so *.% could truncate data.

quoted text
> > > -	memcpy(path.buf, base, baselen);
> > > -	memcpy(path.buf + baselen, filename, filenamelen);
> > > -	path.len = baselen + filenamelen;
> > > -	path.buf[path.len] = '__PLACEHOLDER__1_';
> > > +	strbuf_grow(&path, MAX(PATH_MAX, baselen + filenamelen + 1));
> > > +	strbuf_reset(&path);
> > 
> > Does strbuf_reset() do anything here?
> > 
> > > +	strbuf_add(&path, base, baselen);
> 
>   Yes _reset() sets length to 0. so the add here will write at the start
> of the buffer again. It definitely is important !

But where was length set to non-zero?  path is initialized on entry to
the function, and strbuf_grow() just increases the allocation, not
length, right?

Kristian


-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html