On Fri, Jan 22, 2016 at 1:17 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
> 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.
Sure, I think this would do:
diff --git a/strbuf.c b/strbuf.c
index b552a13..81e279d 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -115,7 +115,7 @@ void strbuf_tolower(struct strbuf *sb)
}
struct strbuf **strbuf_split_buf(const char *str, size_t slen,
- int terminator, int max)
+ int terminator, int max, int omit_term)
{
struct strbuf **ret = NULL;
size_t nr = 0, alloc = 0;
@@ -123,14 +123,18 @@ struct strbuf **strbuf_split_buf(const char
*str, size_t slen,
while (slen) {
int len = slen;
+ int copylen = len;
+ const char *end = NULL;
if (max <= 0 || nr + 1 < max) {
- const char *end = memchr(str, terminator, slen);
- if (end)
+ end = memchr(str, terminator, slen);
+ if (end) {
len = end - str + 1;
+ copylen = len - !!omit_term;
+ }
}
t = xmalloc(sizeof(struct strbuf));
- strbuf_init(t, len);
- strbuf_add(t, str, len);
+ strbuf_init(t, copylen);
+ strbuf_add(t, str, copylen);
ALLOC_GROW(ret, nr + 2, alloc);
ret[nr++] = t;
str += len;
--
Regards,
Karthik Nayak
--
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