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

Re: [PATCH v3 01/15] strbuf: introduce strbuf_split_str_omit_term()

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Jan 21, 2016, 19:47 UTC
Message-ID
<CAPig+cQ2-ddENz4dKsvTW-tOrwugUGa4g-3PySMTBsHsrtO93w@mail.gmail.com>
In-Reply-To
<CAOLa=ZTKiGM1XLr7UuNVX_e_qN4RAGfvOh6daaxcOeZ9xa2TdA@mail.gmail.com>
On Wed, Jan 6, 2016 at 2:27 AM, Karthik Nayak <karthik.188@gmail.com> wrote:
Show 38 quoted lines
> On Wed, Jan 6, 2016 at 12:54 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Karthik Nayak <karthik.188@gmail.com> writes:
>>>       while (slen) {
>>>               int len = slen;
>>> +             const char *end = NULL;
>>>               if (max <= 0 || nr + 1 < max) {
>>> -                     const char *end = memchr(str, terminator, slen);
>>> +                     end = memchr(str, terminator, slen);
>>>                       if (end)
>>>                               len = end - str + 1;
>>>               }
>>>               t = xmalloc(sizeof(struct strbuf));
>>>               strbuf_init(t, len);
>>> -             strbuf_add(t, str, len);
>>> +             strbuf_add(t, str, len - !!end * !!omit_term);
>>
>> Perhaps using another variable would make it easier to follow?
>> Either using a boolean that tells us that the terminating byte
>> is to be omitted, i.e.
>>
>>         int len = slen;
>>         int omit = 0;
>>         if ( ... we are still splitting ... ) {
>>                 const char *end = memchr(...);
>>                 if (end) {
>>                         len = end - str + 1;
>>                         omit = !!omit_term;
>>                 }
>>         }
>>         strbuf_init(t, len - omit);
>>         strbuf_add(t, str, len - omit);
>>
>> or an integer "copylen" that tells us how many bytes to copy, which
>> often is the same as "len" but sometimes different by 1 byte?
>
> This is done based on Eric's suggestion [1]. Although its a little off normal
> convention. I find it small and simple. So I'm okay with either, your suggested
> change or the existing code.

A "copylen" variable would probably result in the clearest code since it states explicitly what an otherwise opaque expression like (!!end * !!omit_term) means, thus is easier to reason about.

Previous: Karthik NayakNext: Karthik Nayak
Message 5 of 52 in “ref-filter: use parsing functions”
  1. 00/15 ref-filter: use parsing functionsKarthik Nayak, Jan 5, 2016
  2. 01/15 strbuf: introduce strbuf_split_str_omit_term()Karthik Nayak, Jan 5, 2016
  3. Junio C HamanoJan 5, 2016
  4. Karthik NayakJan 6, 2016
  5. Eric SunshineJan 21, 2016
  6. Karthik NayakJan 25, 2016
  7. 02/15 ref-filter: use strbuf_split_str_omit_term()Karthik Nayak, Jan 5, 2016
  8. 03/15 ref-filter: bump 'used_atom' and related code to the topKarthik Nayak, Jan 5, 2016
  9. 04/15 ref-filter: introduce struct used_atomKarthik Nayak, Jan 5, 2016
  10. Eric SunshineJan 21, 2016
  11. Karthik NayakJan 25, 2016
  12. 05/15 ref-filter: introduce parsing functions for each valid atomKarthik Nayak, Jan 5, 2016
  13. 06/15 ref-fitler: bump match_atom() name to the topKarthik Nayak, Jan 5, 2016
  14. 07/15 ref-filter: skip deref specifier in match_atom_name()Karthik Nayak, Jan 5, 2016
  15. 08/15 ref-filter: introduce color_atom_parser()Karthik Nayak, Jan 5, 2016
  16. Junio C HamanoJan 5, 2016
  17. Karthik NayakJan 6, 2016
  18. Junio C HamanoJan 5, 2016
  19. Karthik NayakJan 6, 2016
  20. 09/15 ref-filter: introduce align_atom_parser()Karthik Nayak, Jan 5, 2016
  21. 10/15 ref-filter: introduce parse_align_position()Karthik Nayak, Jan 5, 2016
  22. Eric SunshineJan 25, 2016
  23. Karthik NayakJan 26, 2016
  24. 11/15 ref-filter: convert variable 'width' to an unsigned intKarthik Nayak, Jan 5, 2016
  25. Junio C HamanoJan 5, 2016
  26. Karthik NayakJan 6, 2016
  27. 12/15 ref-filter: align: introduce long-form syntaxKarthik Nayak, Jan 5, 2016
  28. Eric SunshineJan 25, 2016
  29. Junio C HamanoJan 25, 2016
  30. Karthik NayakJan 26, 2016
  31. Karthik NayakJan 26, 2016
  32. Christian CouderJan 26, 2016
  33. Karthik NayakJan 26, 2016
  34. 13/15 ref-filter: introduce remote_ref_atom_parser()Karthik Nayak, Jan 5, 2016
  35. Eric SunshineJan 26, 2016
  36. Karthik NayakJan 26, 2016
  37. 14/15 ref-filter: introduce contents_atom_parser()Karthik Nayak, Jan 5, 2016
  38. Junio C HamanoJan 5, 2016
  39. Karthik NayakJan 6, 2016
  40. Junio C HamanoJan 7, 2016
  41. Karthik NayakJan 7, 2016
  42. 15/15 ref-filter: introduce objectname_atom_parser()Karthik Nayak, Jan 5, 2016
  43. Junio C HamanoJan 5, 2016
  44. Karthik NayakJan 6, 2016
  45. Eric SunshineJan 6, 2016
  46. Karthik NayakJan 7, 2016
  47. Junio C HamanoJan 7, 2016
  48. Karthik NayakJan 7, 2016
  49. Eric SunshineJan 7, 2016
  50. Karthik NayakJan 7, 2016
  51. Junio C HamanoJan 7, 2016
  52. Karthik NayakJan 9, 2016

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.