Re: [avr-libc-dev] [ Bug #1720 ] memcmp bug ?

[Theodore A. Roth]

Ok. I just ran this code through simulavr/gdb (should have done this in
the first place):

#include <string.h>

char s1[] = {0xFF,0xFF,0xFF,0xFF,0xFF,0xFF};
char s2[] = {0xFF,0x00,0x00,0x00,0x00,0x00};

int main( void )
{
    volatile int n = 6;
    volatile int res = 0xaa;

    res = memcmp (s1, s2, n);
    if (res < 0)
            n = 1;
    else if (res > 0)
            n = 2;

    return(0);
}

After the memcmp() call, res is 255 and n is 2, as expected.

I'm going to close this bug report and add a note to the dox about
memcmp() not being -mint8 compatible. [see attached patch]

On Mon, 18 Nov 2002, Marek Michalkiewicz wrote:

:) > Regarding:
:) >
:) >   http://savannah.nongnu.org/bugs/?func=detailbug&bug_id=1720&group_id=2140
:) >
:) > Is this really a bug?
:)
:) Both the glibc manual and SUSv2 say that the values compared are
:) interpreted as unsigned char, then the difference promoted to int.
:) So I don't think this is a bug.
:)
:) > I'm leaning toward leaving the behaviour as is, but adding a note in the
:) > dox explaining the signed nature of the compare operation.
:)
:) It's unsigned, actually :)
:)
:) Note that for the example in the bug report:
:)
:) *s1={0xFF,0xFF,0xFF,0xFF,0xFF,0xFF};
:) *s2={0xFF,0x00,0x00,0x00,0x00,0x00};
:)
:) the result is 0x00FF (carry is cleared, so the high byte is 0) and
:) this is correct (positive, because s1[1] > s2[2] when compared as
:) unsigned char).

I assume you mean s1[1] > s2[1] here.

What you say matches what I saw in the simulator.

Unless the submitter of the bug speaks up, I going to assume they did
this:

  char res = memcmp (s1, s1, n);

which would then be an incorrect interpretation of the result.

:)
:) > If the change should be made, I think it would be the following patch
:) > (don't sign extend the result). Is this patch correct?
:)
:) I don't think so.  The sign (from subtracting two "unsigned char"
:) values) is in the carry flag, and _that_ is extended to bits 8-15.
:)
:) Yes, the result is wrong with -mint8, but fixing that is not so
:) simple.  I guess something like this would work (not tested):
:)
    <snip>
:)
:) This way the result is correct as both 8-bit and 16-bit value.
:) Similar changes would be needed for strcmp() and a few others.
:) But, this is 5 more instructions than the current version, so
:) the question how important is to support -mint8 remains...

I think the -mint8 support issue debate is still open. ;-)

Thanks for the input Marek. I'm now convinced this is not a bug.

Ted Roth

memcmp-note.diff
Description: Text document