Re: strbuf API

[Pierre Habouzit <madcoder@debian.org>]

[-- Attachment #1: Type: text/plain, Size: 2759 bytes --]

On Mon, Sep 03, 2007 at 05:43:44AM +0000, Johan Herland wrote:
> On Monday 03 September 2007, Pierre Habouzit wrote:
> >   , and would 
> > patches that would refactor the strbuf module to have those functions,
> > and would fix the code that uses strbuf's to interact properly, be
> > accepted ?
> 
> I don't know. Keep in mind that there is a parallel process (cf. the 
> continuation of the "Buffer overflows" thread) to evaluate the Bstring 
> library ( http://bstring.sourceforge.net/ ), and possibly substitute that 
> for the strbuf "module".
>
> I wouldn't want the work done by you or others to be wasted (depending on 
> which solution wins out in the end), so I suggest you take a look at 
> bstring and offer up arguments why your solution would be better for git.

  Oh I completely missed that the thread was still alive.

> I'd like to know how your solution compare with the alternatives (in this 
> case bstring): What's the performance compared to bstring? How much thought 
> has been given to the usability of the API? What functionality is covered 
> (compared to bstring)? In short: Why is your alternative the best for git?

  Well, technically, I don't mind, I wasn't really pushing this or this
implementation. I had a look at bstring, I dislike the code that looks
pretty ugly, but they almost took the same decisions, so if the
consensus is that bstring gives flexibility, so be it.

  For the record, I dislike the fact that bstring pullutes the b*
namespace, whereas it should use bstr as a prefix everywhere, and some
functions (like bstrcpy, that should definitely be named bstrdup) are
really named the _wrong_ way: you can dislike legacy C string APIs all
the way you want, if you name a function from your foostr library
foostrbar, it better behave the way strbar does in the legacy C API. In
the legacy C, strcpy clobbers the destination, here there is no
destination as bstrcpy is definitely a strdup operation. I only had a
quick look, and maybe it was the sole of the kind, but ... well.

  I also dislike the fact that there is no inline version of the
function that appends C strings, because it means that when you append a
constant string into a bstr, you'll need to write:

  bstrcat(bstr, "foo", sizeof("foo") - 1) or bstrcat(bstr, "foo", 3),
both suck a lot, and with an inline function using strlen, most
compilers will optimize the strlen away, and the code is way more
readable. But again, this is trivial to implement as an extension as
well, so I really don't mind that much either.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]