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

[Mark Millard <markmi%dsl-only.net@localhost>]

On 2017-Apr-10, at 2:51 AM, Nick Hudson <skrll%netbsd.org@localhost> wrote:

> 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

If the non-negative distance between starting addresses
(absolute value of the difference) is at least as large
as the length of the copy, then there is no overlap in
the copy.

r3 is the non-negative distance between the starting
addresses in the corrected netbsd code.

If the non-negative distance between the starting
addresses are less than the length of the copy then
there is an overlap involved in the copy.

I believe the comment is incorrect with "an" and
that it would be correct with "no" instead: i.e.,
that the original change in OR 36512 was correct
for the comment.

===
Mark Millard
markmi at dsl-only.net