Re: [PATCH] blame.c: prepare_lines should not call xrealloc for every line

[Junio C Hamano <gitster@pobox.com>]

David Kastrup <dak@gnu.org> writes:

>> so something like
>>
>> 	for (p = buf; p < end; p++) {
>>         	p = find the end of this line
>>                 if (!p)
>>                 	break;
>> 		num++;
>> 	}
>>
>> perhaps?
>
> Would crash on incomplete last line.

Hmph, even with "if !p"?   From your other message:

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

If you flip the if statement around that would be the same as:

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

And with "loop action not in the control part", that would be the
same as:

	for (p = buf; ; p++) {
		p = memchr...
                if (!p)
                	break;
		num++;
	}

no?  Would this crash on incomplete last line?

Ahh, "p < end" in "for (p = buf; p < end; p++) {" is not correct;
you depend on overrunning the end of the buffer to feed length 0 to
memchr and returning NULL inside the loop.  So to be equivalent to
your version, the contination condition needs to be

	for (p = buf; p <= end; p++) {

But that last round of the loop will be no-op, so "p < end" vs "p <=
end" does not make any difference.  It is not even strictly
necessary because memchr() limits the scan to end, but it would
still be a good belt-and-suspenders defensive coding practice, I
would think.

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

Can we do the same transformation here?

	for (p = buf;;) {
        	*lineno++ = p - buf;
                p = memchr...
                if (!p)
                	break;
		p++;
	}

which is the same as

	for (p = buf; ; p++) {
        	*lineno++ = p - buf;
                p = memchr...
                if (!p)
                	break;
	}

and the latter has the loop control p++ at where it belongs to. The
post-condition of one iteration of the body of the loop being "p
points at the terminating LF of this line", incrementing the pointer
is how the loop control advances the pointer to the beginning of the
next line.

If we wanted to have a belt-and-suspenders safety, we need "p <=
end" here, not "p < end", because of how p is used when it is past
the last LF.  As we want to make these two loops look similar, that
means we should use "p <= end" in the previous loop as well.

This was fun ;-)