Bug in memcmp() routine of stdlib.c

[pturczak]

Hi,

when I wrote an Application for the NiceLite Stack on the M5223EVB calls to memcmp always seemed to return 0. So the test cases used where:
if (memcmp("ABC","ABC",3)==0)
printf("Check 1: OK\r\n");
else
printf("Check 1: Failed\r\n");

if (memcmp("ABC","DEF",3)==0)
printf("Check 2: Failed\r\n");
else
printf("Check 2: OK\r\n");

The EVB said:
Check 1: OK
Check 2: Failed

So basically it would always return 0. So the code of memcmp() in stdlib.c was:
int
memcmp (const void *s1, const void *s2, unsigned n)
{
unsigned char *s1p, *s2p;

if (s1 && s2 && (n > 0))
{
s1p = (unsigned char *)s1;
s2p = (unsigned char *)s2;

while ((--n >= 0) && (*s1p == *s2p))
{
if (*s1p != *s2p)
return (*s1p - *s2p);
++s1p;
++s2p;
}
}

return (0);
}

Note that the expression for the while loop was wrong:
1. --n Will _always_ be >=0 because it is unsigned
2. We should not end the loop by the clause, so the return(0) will be reached.

To Fix the code you should replace the while condition by (n-- != 0). This should fix the bug.

int
memcmp (const void *s1, const void *s2, unsigned n)
{
unsigned char *s1p, *s2p;

if (s1 && s2 && (n > 0))
{
s1p = (unsigned char *)s1;
s2p = (unsigned char *)s2;

while (n-- != 0)
{
if (*s1p != *s2p)
return (*s1p - *s2p);
++s1p;
++s2p;
}
}

return (0);
}

Greetings
Peter

[CompilerGuru]

Yes, looks like a bug in this routine. But I shortly grepped over CodeWarrior for CF 6.3 and I did not find this code.
So my question is were did you take this stdlib.c with the buggy memcmp from?
(also from which version?)

Daniel

[pturczak]

Hi Daniel,

thank you for your fast reply. I found this routine in the code the InterNice Lite Stack distributed on the MCF52235 CD and the Freescale website. It is located in the common directory within the archive. It was imported to our CVS on Tue Aug 8 07:37:27 2006 UTC.

Greetings
Peter

[PaoloRenzo]

Hi

I have just found this issue in our common library  with a completely different code using that stdlib.c file

I will report this asap

Paolo

[bkatt]

Also, strncpy in that file can write 1 character beyond the specified limit;

the 4 str*cmp functions do not treat their data as unsigned char (affects the sign of the nonzero result);

the strchr function does not find '\0' at the end of the string; 

the strtoul function does not always set the **ptr argument to the original *str argument when no valid number is found;

and the isspace function does not return true for \n, \r, \f, and \v.

In printf.c, the code that expands some instances of \n to CRLF should be removed.

[PaoloRenzo]

What strtoul(...) are you referring to? The one that comes with CW libraries?

[bkatt]

---

Paolo Renzo wrote:
What strtoul(...) are you referring to? The one that comes with CW libraries?

---

No, the original poster referred to "stdlib.c" that comes with Niche Lite (in the common folder). He pointed out that memcmp() didn't work at all. I added that several other functions did not behave the way the standards require. Those functions are in stdlib.c, and in one case printf.c.

There is something called FNET from a freescaler named Butok which seems to have the same functions but with some of the problems fixed. His memcmp introduces another flaw (never returns negative).

One other problem: strncpy (in stdlib.c) does not pad out the buffer with zeros.