Skip to content

Commit

Permalink
Usermode/libc #6 Fix string.h functions, add some more unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
thepowersgang committed Jan 12, 2015
1 parent 0abb4e6 commit 7d1c355
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 51 deletions.
7 changes: 2 additions & 5 deletions Usermode/Libraries/Makefile.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ else
V := @
endif

.PHONY: all clean install postbuild
.PHONY: all clean install postbuild utest-build utest-run generate_exp

all: _libs $(_BIN) $(_XBIN)

Expand All @@ -60,16 +60,13 @@ _libs: $(HEADERS)

.PHONY: utest utest-build utest-run $(UTESTS:%=runtest-%)

utest: utest-build utest-run
utest: utest-build generate_exp utest-run

generate_exp: $(UTESTS:%=EXP_%.txt)
@echo > /dev/null

utest-build: $(UTESTS:%=TEST_%)
@echo > /dev/null

utest-run: $(UTESTS:%=runtest-%)
@echo > /dev/null

$(UTESTS:%=runtest-%): runtest-%: TEST_% EXP_%.txt
@echo --- [TEST] $*
Expand Down
4 changes: 2 additions & 2 deletions Usermode/Libraries/libc.so_src/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ endif
include ../Makefile.tpl

EXP_%.txt: TEST_%.native
./$< > $@
rm $<
@./$< > $@
@rm $<
EXP_strtoi.txt:
echo -n "" > $@

Expand Down
35 changes: 35 additions & 0 deletions Usermode/Libraries/libc.so_src/TEST_string.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
*/
#include <stdio.h>
#include <string.h>

#define ASSERT(cnd) printf("ASSERT: "#cnd" == %s\n", ((cnd) ? "pass" : "FAIL"))

int main()
{
ASSERT(strcmp("hello", "world") < 0);
ASSERT(strcmp("hello", "hello") == 0);
ASSERT(strcmp("wello", "hello") > 0);
ASSERT(strcmp("\xff", "\1") > 0);
ASSERT(strcmp("\1", "\xff") < 0);
ASSERT(strcmp("Hello", "hello") < 0);

ASSERT(strncmp("hello world", "hello", 5) == 0);

ASSERT(strcasecmp("hello", "world") < 0);
ASSERT(strcasecmp("hello", "hello") == 0);
ASSERT(strcasecmp("wello", "hello") > 0);
ASSERT(strcasecmp("\xff", "\1") > 0);
ASSERT(strcasecmp("\1", "\xff") < 0);
ASSERT(strcasecmp("Hello", "hello") == 0);
ASSERT(strcasecmp("Hello", "Hello") == 0);
ASSERT(strcasecmp("hellO", "Hello") == 0);


char buf[13];
memset(buf, 0, 13);
ASSERT(buf[0] == 0); ASSERT(buf[12] == 0);

ASSERT(memchr("\xffhello", 'x', 6) == NULL);
}

6 changes: 2 additions & 4 deletions Usermode/Libraries/libc.so_src/stdio.c
Original file line number Diff line number Diff line change
Expand Up @@ -795,13 +795,12 @@ EXPORT char *fgets(char *s, int size, FILE *fp)
*/
EXPORT int fputc(int c, FILE *fp)
{
char ch = c;
unsigned char ch = c;
return fwrite(&ch, 1, 1, fp);

This comment has been minimized.

Copy link
@sortie

sortie Jan 12, 2015

Wrong return value. fputc should return the written byte on success or EOF on failure.

}

EXPORT int putchar(int c)
{
c &= 0xFF;
return fputc(c, stdout);
}

Expand All @@ -811,7 +810,7 @@ EXPORT int putchar(int c)
*/
EXPORT int fgetc(FILE *fp)
{
char ret = 0;
unsigned char ret = 0;
if( fread(&ret, 1, 1, fp) != 1 )
return -1;

This comment has been minimized.

Copy link
@sortie

sortie Jan 12, 2015

This should be the symbolic constant EOF rather than -1.

return ret;
Expand All @@ -825,7 +824,6 @@ EXPORT int getchar(void)

EXPORT int puts(const char *str)
{

if(!str) return 0;
int len = strlen(str);

Expand Down
94 changes: 54 additions & 40 deletions Usermode/Libraries/libc.so_src/string.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* AcessOS Basic C Library
* string.c
*/
#include <acess/sys.h>
//#include <acess/sys.h>
#include <stdlib.h>
#include <stdio.h>
#include <ctype.h>
Expand All @@ -13,20 +13,19 @@
* \fn EXPORT int strcmp(const char *s1, const char *s2)
* \brief Compare two strings
*/
EXPORT int strcmp(const char *s1, const char *s2)
EXPORT int strcmp(const char *_s1, const char *_s2)
{
while(*s1 && *s1 == *s2) {
s1++; s2++;
}
return (int)*s1 - (int)*s2;
return strncmp(_s1, _s2, SIZE_MAX);
}

/**
* \fn EXPORT int strncmp(const char *s1, const char *s2)
* \brief Compare two strings
* \fn EXPORT int strncmp(const char *s1, const char *s2, size_t n)
* \brief Compare two strings, stopping after n characters
*/
EXPORT int strncmp(const char *s1, const char *s2, size_t n)
EXPORT int strncmp(const char *_s1, const char *_s2, size_t n)
{
const unsigned char* s1 = (const unsigned char*)_s1;
const unsigned char* s2 = (const unsigned char*)_s2;
while(n && *s1 && *s1 == *s2)
{
s1++; s2++;
Expand All @@ -38,23 +37,31 @@ EXPORT int strncmp(const char *s1, const char *s2, size_t n)
return (int)*s1 - (int)*s2;
}

EXPORT int strcasecmp(const char *s1, const char *s2)
EXPORT int strcasecmp(const char *_s1, const char *_s2)
{
int rv;
while( (rv = toupper(*s1) - toupper(*s2)) == 0 && *s1 != '\0' && *s2 != '\0' ) {
s1++; s2++;
}
return rv;
return strncasecmp(_s1, _s2, SIZE_MAX);
}

EXPORT int strncasecmp(const char *s1, const char *s2, size_t n)
EXPORT int strncasecmp(const char *_s1, const char *_s2, size_t n)
{
int rv = 0;
if( n == 0 ) return 0;
while(n -- && (rv = toupper(*s1) - toupper(*s2)) == 0 && *s1 != '\0' && *s2 != '\0') {
s1++; s2++;
const unsigned char* s1 = (const unsigned char*)_s1;
const unsigned char* s2 = (const unsigned char*)_s2;
while( n-- && *s1 && *s2 )
{
if( *s1 != *s2 )
{
int rv;
rv = toupper(*s1) - toupper(*s2);
if(rv != 0)
return rv;
rv = tolower(*s1) - tolower(*s2);
if(rv != 0)
return rv;
}
s1 ++;
s2 ++;
}
return rv;
return 0;
}

/**
Expand Down Expand Up @@ -131,7 +138,8 @@ EXPORT size_t strlen(const char *str)
EXPORT size_t strnlen(const char *str, size_t maxlen)
{
size_t len;
for( len = 0; maxlen -- && *str; str ++, len ++ );
for( len = 0; maxlen -- && *str; str ++, len ++ )

This comment has been minimized.

Copy link
@sortie

sortie Jan 12, 2015

size_t len = 0; while ( len < maxlen && str[len] ) len++; return len; would be a more natural pattern.

;
return len;
}

Expand Down Expand Up @@ -171,11 +179,12 @@ EXPORT char *strndup(const char *str, size_t maxlen)
* \fn EXPORT char *strchr(char *str, int character)
* \brief Locate a character in a string
*/
EXPORT char *strchr(const char *str, int character)
EXPORT char *strchr(const char *_str, int character)
{
const unsigned char* str = (const unsigned char*)_str;
for(;*str;str++)

This comment has been minimized.

Copy link
@sortie

sortie Jan 12, 2015

Bug: It's valid to strchr("foo", '\0'); and actually get the nul-byte at the end.

{
if(*str == character)
if( *str == character )

This comment has been minimized.

Copy link
@sortie

sortie Jan 12, 2015

You should also truncate character to an unsigned char here.

return (char*)str;
}
return NULL;
Expand All @@ -185,11 +194,10 @@ EXPORT char *strchr(const char *str, int character)
* \fn EXPORT char *strrchr(char *str, int character)
* \brief Locate the last occurance of a character in a string
*/
EXPORT char *strrchr(const char *str, int character)
EXPORT char *strrchr(const char *_str, int character)
{
int i;
i = strlen(str)-1;
while(i--)
const unsigned char* str = (const unsigned char*)_str;
for( int i = strlen(_str); i--; )

This comment has been minimized.

Copy link
@sortie

sortie Jan 12, 2015

int should be size_t here. Alternative, it's probably better to just loop one time forward through the string and remember the last occurrance of the character.

This comment has been minimized.

Copy link
@sortie

sortie Jan 12, 2015

The nul byte is part of the string and can be searched for, this ignores it.

{
if(str[i] == character)

This comment has been minimized.

Copy link
@sortie

sortie Jan 12, 2015

You should also truncate character to an unsigned char here.

return (void*)&str[i];

This comment has been minimized.

Copy link
@sortie

sortie Jan 12, 2015

Did you mean a cast to char*?

Expand Down Expand Up @@ -277,6 +285,8 @@ EXPORT void *memcpy(void *__dest, const void *__src, size_t count)
return __dest;
}

// TODO: memccpy (POSIX defined)

/**
* \fn EXPORT void *memmove(void *dest, const void *src, size_t count)
* \brief Copy data in memory, avoiding overlap problems
Expand Down Expand Up @@ -316,7 +326,7 @@ EXPORT int memcmp(const void *mem1, const void *mem2, size_t count)
while(count--)
{
if( *p1 != *p2 )
return *p1 - *p2;
return (int)*p1 - (int)*p2;
p1 ++;
p2 ++;
}
Expand All @@ -332,24 +342,26 @@ EXPORT int memcmp(const void *mem1, const void *mem2, size_t count)
*/
EXPORT void *memchr(const void *ptr, int value, size_t num)
{
const unsigned char* buf = ptr;
while(num--)
{
if( *(const unsigned char*)ptr == (unsigned char)value )
return (void*)ptr;
ptr ++;
if( *buf == (unsigned char)value )
return (void*)buf;
buf ++;
}
return NULL;
}

EXPORT size_t strcspn(const char *haystack, const char *reject)
{
size_t ret = 0;
int i;
while( *haystack )
{
for( i = 0; reject[i] && reject[i] == *haystack; i ++ );

if( reject[i] ) return ret;
for( int i = 0; reject[i]; i ++ )

This comment has been minimized.

Copy link
@sortie

sortie Jan 12, 2015

Use size_t instead of int.

{
if( reject[i] == *haystack )
return ret;
}
ret ++;

This comment has been minimized.

Copy link
@sortie

sortie Jan 12, 2015

haystack is never incremented. I suggest using haystack[ret] instead of *haystack.

}
return ret;
Expand All @@ -358,12 +370,13 @@ EXPORT size_t strcspn(const char *haystack, const char *reject)
EXPORT size_t strspn(const char *haystack, const char *accept)
{
size_t ret = 0;
int i;
while( *haystack )
{
for( i = 0; accept[i] && accept[i] == *haystack; i ++ );

if( !accept[i] ) return ret;
for( int i = 0; accept[i]; i ++ )
{
if( accept[i] != *haystack )

This comment has been minimized.

Copy link
@sortie

sortie Jan 12, 2015

This is wrong. strspn("b", "ab") gives the wrong result, 0 instead of 1 because it stops too early. It needs to check the entire accept string before deciding this character in haystack isn't in accept.

return ret;
}
ret ++;

This comment has been minimized.

Copy link
@sortie

sortie Jan 12, 2015

haystack is never incremented. I suggest using haystack[ret] instead of *haystack.

}
return ret;
Expand All @@ -378,6 +391,7 @@ EXPORT char *strpbrk(const char *haystack, const char *accept)
if( accept[i] == *haystack )

This comment has been minimized.

Copy link
@sortie

sortie Jan 12, 2015

Use size_t instead of int. This can be implemented in terms of strcspn, btw.

return (char*)haystack;
}
haystack ++;
}
return NULL;
}
Expand Down

0 comments on commit 7d1c355

Please sign in to comment.