On Thu, 2007-09-06 at 20:08 +0200, Pierre Habouzit wrote: > 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 \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. > > > - memcpy(path.buf, base, baselen); > > > - memcpy(path.buf + baselen, filename, filenamelen); > > > - path.len = baselen + filenamelen; > > > - path.buf[path.len] = '\0'; > > > + 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
