Re: Latest builtin-commit series

[Pierre Habouzit <madcoder@debian.org>]

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

On Tue, Sep 18, 2007 at 03:23:29PM +0000, Kristian Høgsberg wrote:
>       * rebase to Pierres strbuf changes.  Note, there is still some
>         strbuf tweaking required, to let stripspace work on a strbuf.

  Yeah I wondered if that would be a gain to migrate stripspace as a
first class citizen strbuf function. Note though, that if you always
want to stripspace in place, changing it is an overkill. I mean, there
isn't a lot of gain, as making it work on a strbuf is just a matter of:

    strbuf_setlen(&buf, stripspaces(buf.buf, buf.len));

If you want to do sth like:

    stripspaces(&buf, some_other_string);

And see the stripped version of "some_other_string" appended to the
strbuf "buf", then yes, it's not trivial to use the current stripspaces
as is.


General Note:
~~~~~~~~~~~~

  As a general rule, and I say this to the list, not only to you,
strbufs should not be used everywhere. It may _look_ like I'm peeing
over all the code putting strbufs anywhere I can, it's not true.
Strbuf's can help for two things:
  * the obvious: dealing with variable length strings, it's the least
    they can do.
  * be used as reused, variable length buffer, instead of loops that do:
      for (;;) {
         char *foo = xmalloc(...);
         [...]
         free(foo);
      }
    here, you can have a strbuf outside of the loop, and just reset it
    at the begining of the loop. You'll then work on a buffer that will
    stop beeing reallocated at some point. This make allocation patterns
    better.

  Strbuf's are not especially convenient when it comes to parsing.
Unlike the bstring's that have been discussed here recently, I don't
mean strbuf's to supersede all the standard C string API, because when
it comes to parsing, as soon as your buffers are properly NUL
terminated, C functions are _not_ usafe, no matter what people say.
strchr/memchr, strc?spn, strn?cmp/memcmp, ... are very efficient, and
there is little point to hide them behind stupid strbuf's APIs. Hence,
when it comes to parsing, you just fallback to a string, whose length is
known[0].

  That's the reason why I won't probably be the one converting
builtin-mailinfo.c to strbuf's because there is very little point to do
so in the current state of the art.


Cheers,


  [0] that allows to fallback to memchr[1] instead of strchr, which is
      slightly faster I'm told.

  [1] Note that you must be sure you don't have embedded NULs or memchr
      and strchr won't have the same result then ;)
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

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