Junio C Hamano <gits...@pobox.com> writes:

> David Kastrup <d...@gnu.org> writes:
>
>> Making a single preparation run for counting the lines will avoid memory
>> fragmentation.  Also, fix the allocated memory size which was wrong
>> when sizeof(int *) != sizeof(int), and would have been too small
>> for sizeof(int *) < sizeof(int), admittedly unlikely.
>>
>> Signed-off-by: David Kastrup <d...@gnu.org>
>> ---
>>  builtin/blame.c | 40 ++++++++++++++++++++++++----------------
>>  1 file changed, 24 insertions(+), 16 deletions(-)
>>
>> diff --git a/builtin/blame.c b/builtin/blame.c
>> index e44a6bb..522986d 100644
>> --- a/builtin/blame.c
>> +++ b/builtin/blame.c
>> @@ -1772,25 +1772,33 @@ static int prepare_lines(struct scoreboard *sb)
>>  {
>>      const char *buf = sb->final_buf;
>>      unsigned long len = sb->final_buf_size;
>> -    int num = 0, incomplete = 0, bol = 1;
>> +    const char *end = buf + len;
>> +    const char *p;
>> +    int *lineno;
>> +    
>> +    int num = 0, incomplete = 0;
>
> Is there any significance to the blank line between these two
> variable definitions?
>
>> +
>> +    for (p = buf;;) {
>> +            if ((p = memchr(p, '\n', end-p)) == NULL)
>> +                    break;
>> +            ++num, ++p;
>
> You have a peculiar style that is somewhat distracting.  Why isn't
> this more like so?
>
>       for (p = buf; p++, num++; ) {
>               p = memchr(p, '\n', end - p);
>               if (!p)
>                       break;
>       }

Ok, I now wrote

        for (p = buf;; num++, p++) {
                p = memchr(p, '\n', end - p);
                if (!p)
                        break;
        }

and you know what?  Its logic seems wrong.  The reason is that the p++
does not really have anything to do with the iteration, but rather steps
past the '\n' from the memchr.  So it's more like

        for (p = buf;; num++) {
                p = memchr(p, '\n', end - p);
                if (p) {
                        p++;
                        continue;
                }
                break;
        }

So barring protests, that's what I'm going to use instead.

-- 
David Kastrup

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to