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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jan 6, 2016 at 2:27 AM, Karthik Nayak <karthik.188@xxxxxxxxx> wrote:
> On Wed, Jan 6, 2016 at 12:54 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> Karthik Nayak <karthik.188@xxxxxxxxx> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]