git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: Latest builtin-commit series

From
Pierre Habouzit <madcoder@debian.org>
Date
Sep 18, 2007, 22:31 UTC
Message-ID
<20070918223119.GA4535@artemis.corp>
In-Reply-To
<1190129009.23692.24.camel@hinata.boston.redhat.com>
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
Previous: Kristian HøgsbergNext: Junio C Hamano
Message 6 of 7 in “Latest builtin-commit series”
  1. Kristian HøgsbergSep 18, 2007
  2. Alex RiesenSep 18, 2007
  3. Johannes SchindelinSep 18, 2007
  4. Junio C HamanoSep 18, 2007
  5. Kristian HøgsbergSep 18, 2007
  6. Pierre HabouzitSep 18, 2007
  7. Junio C HamanoSep 19, 2007

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.