Why does my implementation of strlcat gives a segmentation fault when the original does not?

Emperor_Udan 0
Viewed 84

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.

segmentation-fault function c
2 Answers

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

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

s1[] too small for the concatenated string "HelloWorld"

char s1[11 /* or more */] = "Hello"; // Use a string

"%lu" matches unsigned long. size_t from sizeof matches "%zu".

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)

Some untested code for your consideration:

  • Tries to mimic strlcat().

  • Returns sum of string lengths, but not more that size.

  • Does not examine more than size characters to prevent reading out of bounds.

  • Does not append a null character when not enough room.

  • Does not check for dst, src as NULL. Add if you like.

  • Does not handle overlapping dest, src. To do so is tricky unless library routines available.

  • Use unsigned char * pointer to properly handle rare signed non-2's complement char.

size_t my_strlcat(char * restrict dst, const char * restrict src, size_t size) {
  const size_t size_org = size;

  // Walk dst
  unsigned char *d = (unsigned char*) dst;
  while (size > 0 && *d) {
    d++;
    size--;
  }
  if (size == 0) {
    return size_org;
  }

  // Copy src to dst
  const unsigned char *s = (const unsigned char*) src;
  while (size > 0 && *s) {
    *d++ = *s++;
    size--;
  }
  if (size == 0) {
    return size_org;
  }

  *d = '\0';
  return (size_t) (d - (unsigned char*) dst);
}

If the return value is less than size, success!

chux - Reinstate Monica0

  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;
}
  1. 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

0___________0
Related