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.