On Fri, Oct 05, 2007 at 06:35:17PM +0200, Pierre Habouzit wrote: > On Fri, Oct 05, 2007 at 04:21:39PM +0000, Sam Ravnborg wrote: > > On Fri, Oct 05, 2007 at 08:26:44AM -0700, Linus Torvalds wrote: > > > > > > > > > On Fri, 5 Oct 2007, Pierre Habouzit wrote: > > > > > > > > - strbuf_grow(buf, len); > > > > + /* only grow if not in place */ > > > > + if (strbuf_avail(buf) + buf->len < len) > > > > + strbuf_grow(buf, len - buf->len); > > > > > > Umm. This is really ugly. > > > > > > The whole point of strbuf's was that you shouldn't be doing your own > > > allocation decisions etc. So why do it? > > > > > > Wouldn't it be much better to have a strbuf_make_room() interface that > > > just guarantees that there is enough room fo "len"? > > > > > > Otherwise, code like the above would seem to make the whole point of a > > > safer string interface rather pointless. The above code only makes sense > > > if you know how the strbuf's are internally done, so it should not exists > > > except as internal strbuf code. No? > > > > Took a short look at strbuf.h after seeing the above code. > > And I was suprised to see that all strbuf users were exposed to > > the strbuf structure. > > Following patch would at least make sure noone fiddle with strbuf internals. > > Cut'n'paste - only for the example of it. > > It simply moves strbuf declaration to the .c file where it rightfully belongs. > > you're looking at an antiquated version, please look at the one in > current master on current next. In this one, what you can do or not do > with the struct is explained > > > git did not build with this change.... > > Of course it doesn't! people want to have direct access to ->buf and > ->len, and it's definitely OK. Understood now - thanks for the clarification. Sam - 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
