0

I am currently recoding srtcat() from the standard C library and I have set up some checks to avoid overlap problems. The problem is that my program still enters the error handling.

Here is the code:

char *my_strcat(char *restrict dest, const char *restrict src) { size_t dest_len = 0, src_len = 0; char *p = dest; src_len = my_strlen(src); if (!dest || !src) return NULL; dest_len = my_strlen(dest); if (src >= dest && src < dest + dest_len) { return NULL; } if (dest >= src && dest < src + src_len) { return NULL; } while (*p != '\0') p++, dest_len++; if (dest_len + src_len + 1 > sizeof(dest)) return NULL; p = dest + dest_len; while (*src != '\0') *p++ = *src++; *p = '\0'; return dest; } 
size_t my_strlen(const char *s) { size_t count = 0; if (s != NULL) { while (*s != 0) { count++; s++; } } return count; }

I tested this way :

int main(int argc, char **argv) { const char *src = "Hello"; char dest[100] = " world!"; char *test = my_strcat(dest, src); printf("Src : %s Dest : %s\n", src, dest); printf("Return adress : %p, Value : %s\n", test, test); return 0; }

According to gdb :

if (src >= dest && src < dest + dest_len) 1: dest = 0x7fffffffda70 " world!" 2: src = 0x555555557004 "Hello" 3: dest_len = 0 4: src_len = 5

Output

Src : Hello Dest : world! Return adress : (nil), Value : (null)

Do you see the problem?

Update

Following your suggestions I have modified the code like this:

char *my_strcat(char *restrict dest, const char *restrict src, size_t d_size) { size_t dest_len = 0, src_len = 0; char *p = dest; if (!dest || !src) return NULL; src_len = my_strlen(src); dest_len = my_strlen(dest); if (src >= dest && src < dest + dest_len) { return NULL; } if (dest >= src && dest < src + src_len) { return NULL; } while (*p != '\0') p++, dest_len++; if (dest_len + src_len + 1 > d_size) return NULL; p = dest + dest_len; while (*src != '\0') *p++ = *src++; *p = '\0'; return dest; }

And in the main : char *test = my_strcat(dest, src, sizeof(dest));

But it still doesn't work :

Src : Hello Dest : world! Return adress : 0x7fff74bc5650, Value : world!
Improve this question
8
  • 1
    sizeof(dest) in the function measures the size of the pointer. The function must be told by the caller the size of the destination array (in a 3rd parameter). The caller can see the array and sizeof makes sense (as long as the elements are only 1 byte.) The function, as is, only receives the address of the first byte; nothing more... Commented Dec 28, 2022 at 1:31
  • Yes but I want to keep the same prototype as strcat so is there any other way? Commented Dec 28, 2022 at 1:34
  • 1
    In a word, no. This is why strcat() can be dangerous if used indiscriminately. When the function only gets pointers (addresses), it's up to the caller to make sure there's room in the destination array. (C's strings are null-terminated. You could, if you can, define some other "sentinel" to mark the absolute end of dest, but this would be considered "hacky" and would make one more special byte value that cannot appear as ordinary data. Commented Dec 28, 2022 at 1:39
  • The check if (!dest || !src) should be place before src_len = my_strlen(src); Commented Dec 28, 2022 at 1:40
  • Okay, I modified it but it still doesn't work, the only difference is that now I have : Return adress : 0x7ffc573e3db0, Value : world! Commented Dec 28, 2022 at 1:46

2 Answers 2

Reset to default
2

Having tried to guide toward understanding this problem, it seems best to present what should be working code (for study.) Sometimes too many words merely muddle the situation:

char *my_strcat(char *restrict dest, const char *restrict src, size_t d_size) { if( !dest || !src ) return NULL; size_t src_len = strlen( src ); size_t dest_len = strlen( dest ); if( dest_len + src_len + 1 > d_size ) return NULL; char *p = dest + dest_len; while( (*p++ = *src++ ) != '\0' ) ; return dest; } int main() { const char *src = "Hello"; char dest[100] = " world!"; printf("Src : %s Dest : %s\n", src, dest); char *test = my_strcat( dest, src, sizeof dest ); if( test ) printf("Value : %s\n", test ); return 0; }

Now, one can experiment by shrinking the size of dest to something larger than " world!" but smaller than " world!Hello"... Perhaps 9 bytes???

And, now that the concatenation should be working (into a big enough buffer), adding the code to ensure there is no overlap of the actual character arrays. Known is the size of dest, and the length of src is measured.

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

0 
dest_len + src_len + 1 > sizeof(dest)

sizeof(dest) is the size of the pointer sizeof(char*). If you want to check if the dest will be overflowed, you have to pass the size as an argument. See strlcpy or strncpy.

src >= dest

Note that comparing pointers that do not point to the same array is technically invalid. To bring some breeze of validity, you can do (uintptr_t)stc >= (uintptr_t)dest. How does pointer comparison work in C? Is it ok to compare pointers that don't point to the same array? Is comparing two pointers with < undefined behavior if they are both cast to an integer type? Why does comparing pointers with undefined behavior still give correct results? etc.

Improve this answer

1 Comment

I modified it like you told me but it still doesn't work. Just that now the function returns a valid pointer to dest remains unchanged.