I am writing a re-implementation of strlcat as an exercise. I have perform several tests and they produce similar result. However on one particular case, my function gives an segmentation fault error while the original does not, could you explain to me why? I am not allowed to use any of the standard library function, that is why I have re-implemented strlen().

Here is the code I have written :

#include <stdio.h>
#include <string.h>

int ft_strlen(char *s)
{
    int i;

    i = 0;
    while (s[i] != '\0')
        i  ;
    return (i);
}

unsigned int    ft_strlcat(char *dest, char *src, unsigned int size)
{
    size_t  i;
    int     d_len;
    int     s_len;

    i = 0;
    d_len = ft_strlen(dest);
    s_len = ft_strlen(src);
    if (!src || !*src)
        return (d_len);
    while ((src[i] && (i < (size - d_len - 1))))
    {
        dest[i   d_len] = src[i];
        i  ;
    }
    dest[i   d_len] = '\0';
    return (d_len   s_len);
}

int main(void)
{
    char s1[5] = "Hello";
    char s2[] = " World!";

printf("ft_strcat :: %s :: %u :: sizeof %lu\n", s1, ft_strlcat(s1, s2, sizeof(s1)), sizeof(s1));
    // printf("strlcat :: %s :: %lu :: sizeof %lu\n", s1, strlcat(s1, s2, sizeof(s1)), sizeof(s1));
}

The output using strlcat is : strlcat :: Hello World! :: 12 :: sizeof 5. I am on macOS and I am using clang to compile if that can be of some help.

CodePudding user response：

ft_strlcat() is not so bad, but it expects pointers to strings and s1 lacks a null character: so s1 is not a string.

//char s1[5] = "Hello";
char s1[] = "Hello"; // Use a string

Some ft_strlcat() issues:

unsigned, int vs. size_t

unsigned, int too narrow for long strings. Use size_t to handle all strings.

Test too late

if (!src || ...) is too late as prior ft_strlen(src); invokes UB when src == NULL.

const

ft_strlcat() should use a pointer to const to allow usage with const strings with src.

Advanced: restrict

Use restrict so the compiler can assume dest, src do not overlap and emit more efficient code - assuming they should not overlap.

Corner cases

It does not handle some pesky corner cases like when d_len >= size, but I will leave that detailed analysis for later.

Suggested signature

// unsigned int    ft_strlcat(char *dest, char *src, unsigned int size)
size_t ft_strlcat(char * restrict dest, const char * restrict src, size_t size)

CodePudding user response：

1. s1 is not even long enough to accommodate the "Hello"

2. Use the correct type for sizes.

size_t ft_strlcat(char *dest, const char *src, size_t len)
{
    char *savedDest = dest;
    if(dest && src && len)
    {
        while(*dest && len)
        {
            len--;
            dest  ;
        }
        if(len)
        {
            while((*dest = *src) && len)
            {
                len--;
                dest  ;
                *src  ;
            }
        }
        if(!len) dest[-1] = 0;
    }
    return dest ? dest - savedDest : 0;
}   

3. Also your printf invokes undefined behaviour as order of function parameters evaluation is not determined. It should be:

int main(void)
{
    char s1[5] = "Hello";     //will only work for len <= sizeof(s1) as s1 is not null character terminated
    char s2[] = " World!";

    size_t result = ft_strlcat(s1, s2, sizeof(s1));
    printf("ft_strcat :: %s ::  %zu :: sizeof %zu\n", s1, result, sizeof(s1));
}

https://godbolt.org/z/8hhbKjsbx

CodePudding user response：

As indicated above:

1. dest is not initially \0 terminated which means the ft_strlen() will be undefined behavior.
2. dest is insufficiently large to hold the result

Here's the minimal fix for the segfault:

char s1[sizeof("Hello World!")] = "Hello";

You may also want to consider:

3. What should it return for dest being null?
4. Mixing signed and unsigned cause trouble. ft_strlcat(s1, s2, 0) result in Hello World! which doesn't seem right. What should the behavior be for size < d_len 1? Do you truncate dest?