Re: An old arm memove.S patch's comment update was incompletely applied; result is confusing and wrong (tied to PR 36512)

[Nick Hudson <skrll%netbsd.org@localhost>]

On 04/07/17 05:03, Mark Millard wrote:
> [Going to http://www.netbsd.org/cgi-bin/sendpr.cgi?gndb=netbsd I
> ended up unsure how to properly submit a bug for an incorrect/
> misleading comment update that was made in an old patch.]
>
> [For reference:
>   http://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=36512
> ]
>
> The comment issue below was noticed in the activity of noticing
> FreeBSD has the older style of code structure for the relevant
> part of memmove.S. The issue has contributed to confusions for
> the FreeBSD activity. FreeBSD still has the older type of arm
> code structure involved and a couple of folks have been trying
> to get the same type of code-correction in place.
>
> The original change listed in netbsd's PR 36512 listed (note the
> comment's "an" vs. "old"):
>
> -	cmp	r3, r2		/* if (r3 < len) we have an overlap */
> vs.
> +	cmp	r3, r2		/* if (r3 >= len) we have no overlap */
>
> Looking at it I agree with the comment update being accurate
> to the updated code. But the actual source shown in the likes of:
>
> http://cvsweb.netbsd.org/bsdweb.cgi/src/common/lib/libc/arch/arm/string/memmove.S.diff?r1=1.1&r2=1.2
>
> (with r2=1.2 or later) shows:
>
> +	cmp	r3, r2		/* if (r3 >= len) we have an overlap */
>
> (still "an"). So the official comment is misleading by being
> incorrect from what I understand.

I think

	/* if (r3 >= len) we have an overlap */

is correct

Nick