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

Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.

From
Pierre Habouzit <madcoder@debian.org>
Date
Sep 4, 2007, 23:01 UTC
Message-ID
<20070904230117.GA12448@olympe.madism.org>
In-Reply-To
<20070904213857.GA21351@steel.home>
On mar, sep 04, 2007 at 09:38:57 +0000, Alex Riesen wrote:
Show 10 quoted lines
> Lukas Sandström, Tue, Sep 04, 2007 22:50:08 +0200:
> > Hi.
> > 
> > This is an attempt to use "The Better String Library"[1] in builtin-mailinfo.c
> > 
> > The patch doesn't pass all the tests in the testsuit yet, but I thought I'd
> > send it out so people can decide if they like how the code looks.
> 
> It looks uglier, but what are measurable merits? Object code size,
> perfomance hit/improvement, valgrind logs?
  Well I honestly believe that putting strbufs/bstrings in mailinfo.c
adds no value. I was going to give it a try to see how strbufs
performed, but it's just useless.
  The main problem mailinfo has, it's according to Junio that it may
sometimes truncate some things in buffers at 1000 octets, without dying
loudly. That is bad.
  _but_ there is no point in using arbitrary long string buffers to
parse a mail. Remember, a mail goes through SMTP, and SMTP is supposed
to limit its lines at 512 characters (without use of extensions at
least). Not to mention that an email address cannot be more than 64+256
chars long (or sth around that). So using variable lengths buffers is
just a waste.
  string buffers are not really (IMHO) supposed to help in parsing
tasks, and when you need to do some serious parsing, either do it by
hand or use lex, but nothing in between makes sense to me.
  OTOH, string buffers can be used in many places where git has (at
least 4 different to my current count, growing) many implementations of
always slightly different kind of buffers. I've some more patches
pending here than the one I already sent, and well, here is the
diffstat:
$ git diff --stat origin/master.. ^strbuf*
 archive-tar.c         |   67 ++++++++++++------------------------------------
 builtin-apply.c       |   29 ++++++---------------
 builtin-blame.c       |   34 ++++++++-----------------
 builtin-commit-tree.c |   59 +++++++++---------------------------------
 builtin-rerere.c      |   53 +++++++++++---------------------------
 cache-tree.c          |   57 ++++++++++++++---------------------------
 diff.c                |   25 ++++++------------
 fast-import.c         |   38 +++++++++++----------------
 mktree.c              |   26 ++++++-------------
 9 files changed, 116 insertions(+), 272 deletions(-)
  I mean, there is not even a need to show the diff to understand what
the gain is. And that was possible, because strbufs are straightforward,
and gives you the kind of controls git needs (tweaking how memory will
be allocated to avoid reallocs is part of the answer).
  A French author once said: “Il semble que la perfection soit atteinte
non quand il n'y a plus rien à ajouter, mais quand il n'y a plus rien à
retrancher.” -- Antoine de St Éxupéry[0]. IMHO git will never need any
of the bstring splits, streaming functions, tokenization or whatever,
and supporting those has necessarily led the bstring library to make
some choices that may not fit git needs. I don't really like reinventing
the wheel, but OTOH buffers and strings are often of the critical path,
and having a nice fitting buffer API is priceless.
  [0] Perfection is achieved, not when there is nothing more to add, but
      when there is nothing left to take away.
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org
Previous: Alex RiesenNext: Kristian Høgsberg
Message 3 of 102 in “[RFC] Convert builin-mailinfo.c to use The Better String Library.”
  1. Lukas SandströmSep 4, 2007
  2. Alex RiesenSep 4, 2007
  3. Pierre HabouzitSep 4, 2007
  4. Kristian HøgsbergSep 5, 2007
  5. Matthieu MoySep 5, 2007
  6. Miles BaderSep 6, 2007
  7. Dmitry KakurinSep 6, 2007
  8. Shawn O. PearceSep 6, 2007
  9. Andreas EricssonSep 6, 2007
  10. Junio C HamanoSep 6, 2007
  11. Andreas EricssonSep 6, 2007
  12. David KastrupSep 6, 2007
  13. Miles BaderSep 6, 2007
  14. Johannes SchindelinSep 6, 2007
  15. Linus TorvaldsSep 6, 2007
  16. Dmitry KakurinSep 7, 2007
  17. Linus TorvaldsSep 7, 2007
  18. Dmitry KakurinSep 7, 2007
  19. Linus TorvaldsSep 7, 2007
  20. Dmitry KakurinSep 7, 2007
  21. David SymondsSep 7, 2007
  22. Theodore TsoSep 7, 2007
  23. Steven BurnsSep 20, 2007
  24. Andreas EricssonSep 20, 2007
  25. Andreas EricssonSep 7, 2007
  26. Dmitry KakurinSep 7, 2007
  27. David KastrupSep 7, 2007
  28. Dmitry KakurinSep 8, 2007
  29. David KastrupSep 8, 2007
  30. Andreas EricssonSep 9, 2007
  31. David KastrupSep 7, 2007
  32. Johannes SchindelinSep 7, 2007
  33. Johannes SchindelinSep 7, 2007
  34. David KastrupSep 7, 2007
  35. Linus TorvaldsSep 7, 2007
  36. alanSep 7, 2007
  37. Walter BrightSep 7, 2007
  38. David KastrupSep 7, 2007
  39. Walter BrightSep 7, 2007
  40. David KastrupSep 7, 2007
  41. Walter BrightSep 7, 2007
  42. David KastrupSep 7, 2007
  43. Walter BrightSep 7, 2007
  44. David KastrupSep 7, 2007
  45. Walter BrightSep 7, 2007
  46. Andreas EricssonSep 8, 2007
  47. Pierre HabouzitSep 9, 2007
  48. Andreas EricssonSep 9, 2007
  49. Wincent ColaiutaSep 7, 2007
  50. Pierre HabouzitSep 7, 2007
  51. Walter BrightSep 7, 2007
  52. David KastrupSep 7, 2007
  53. Walter BrightSep 7, 2007
  54. Pierre HabouzitSep 7, 2007
  55. David KastrupSep 7, 2007
  56. Pierre HabouzitSep 7, 2007
  57. Walter BrightSep 7, 2007
  58. Pierre HabouzitSep 7, 2007
  59. Walter BrightSep 7, 2007
  60. John 'Z-Bo' ZabroskiSep 8, 2007
  61. David KastrupSep 8, 2007
  62. Steven BurnsSep 19, 2007
  63. Wincent ColaiutaSep 7, 2007
  64. Paul WankadiaSep 7, 2007
  65. Nicolas PitreSep 7, 2007
  66. Wincent ColaiutaSep 7, 2007
  67. Andreas EricssonSep 7, 2007
  68. Johannes SchindelinSep 7, 2007
  69. Andreas EricssonSep 7, 2007
  70. Wincent ColaiutaSep 7, 2007
  71. Karl HasselströmSep 7, 2007
  72. Andreas EricssonSep 7, 2007
  73. Wincent ColaiutaSep 7, 2007
  74. Andreas EricssonSep 9, 2007
  75. David KastrupSep 7, 2007
  76. Wincent ColaiutaSep 7, 2007
  77. Walter BrightSep 7, 2007
  78. Andreas EricssonSep 7, 2007
  79. Walter BrightSep 7, 2007
  80. David KastrupSep 7, 2007
  81. Andreas EricssonSep 9, 2007
  82. Bernd JendrissekSep 17, 2009
  83. Wincent ColaiutaSep 7, 2007
  84. Walter BrightSep 7, 2007
  85. Steven BurnsSep 22, 2007
  86. David KastrupSep 7, 2007
  87. Andy ParkinsSep 7, 2007
  88. David KastrupSep 7, 2007
  89. Johannes SchindelinSep 7, 2007
  90. Dmitry KakurinSep 8, 2007
  91. David KastrupSep 8, 2007
  92. Alex RiesenSep 8, 2007
  93. figoSep 24, 2007
  94. David KastrupSep 24, 2007
  95. Steven BurnsSep 25, 2007
  96. David KastrupSep 25, 2007
  97. Syed M RaihanMay 22, 2012
  98. Ian MoltonJun 10, 2010
  99. Jakub NarebskiJun 11, 2010
  100. Dario RodriguezJun 11, 2010
  101. Kristian HøgsbergSep 5, 2007
  102. Lukas SandströmSep 7, 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.