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