8

I'm working on the implementation of strdup() in C, I'm new to C and I'm trying to build my own library. I'm using the actual strdup() from string.h to test mine and I run into a bus error when I test my source against a main program but none when I use the actual strdup(). Also I can't use any existing functions excluding malloc().

Source code:

#include <stdlib.h>

char *ft_strdup(char *src)
{

    char *str;
    int len;

    while (src[len])
        len++;
    str = (char*)malloc(sizeof(*str) * (len+1));
    while (*src)
        *str++ = *src++;
    *str = '\0';
    return (str);
}

Why do I keep getting a bus error?

adot
  • 398
  • 1
  • 3
  • 14
  • 6
    You miss to initialize `len` with 0 – Ctx May 10 '16 at 08:04
  • 1
    *I'm new to C and I'm trying to build my own library* -- Generally not a good idea. I had been using C for many years before [I started my own standard library](http://pdclib.e43.eu/), and I'm still at it many years later. The `` functions are quite easy, generally speaking, but you will rapidly run into problems (or implement the functions too naively, leaving you with a frustratingly broken project). Try something on the user side of the standard lib for your first steps. – DevSolar May 10 '16 at 08:21
  • @user3121023 Thanks, How do i return the pointer back to the beginning? – adot May 10 '16 at 08:33
  • @user3121023 Nevermind, Thanks for everything ive cracked it. – adot May 10 '16 at 08:35
  • strdup() is not part of the standard library, so there are situations (like porting code that depends on it to other systems), where you need to implement it yourself. – Enno May 10 '16 at 09:39
  • Enable compiler warnings. – Lundin May 10 '16 at 14:28
  • On allocation error `strdup()` is supposed to return `NULL` and set `errno` to `ENOMEM`, not crash with a segmentation fault (if you're on a platform with memory protection, if not, the function will do worse things). – Patrick Schlüter May 10 '16 at 15:51

2 Answers2

19

Try following fix:

  1. Initialize len before increment it.
  2. Don't cast malloc's return value, and don't use sizeof(char), it's defined to be 1 in the standard, per cstd 6.5.3.4p4:

    When sizeof is applied to an operand that has type char, unsigned char, or signed char, (or a qualified version thereof) the result is 1.

  3. Use a pointer to save the original str pointer
#include <stdlib.h>

char *ft_strdup(char *src)
{
    char *str;
    char *p;
    int len = 0;

    while (src[len])
        len++;
    str = malloc(len + 1);
    p = str;
    while (*src)
        *p++ = *src++;
    *p = '\0';
    return str;
}
Community
  • 1
  • 1
fluter
  • 13,238
  • 8
  • 62
  • 100
  • Can you please explain point 2? With regards to the casting of malloc and use of sizeof? – adot May 10 '16 at 08:43
  • @afullstopdot Sure, I've updated the post for the reason of point 2. – fluter May 10 '16 at 08:50
  • I literally started using stackoverflow an hour ago, I didnt know i could do that. @machine_1. I will now. – adot May 10 '16 at 09:10
  • 1
    Note that `*p++ = *src++` is equivalent to `*(p++) = *(src++)` because the postfix operator `++` has higher precedence than the dereference operator `*`. – hexicle May 24 '18 at 00:52
  • fluter that wasthe answer that I was looking for. Thanks. – Jonadabe Sep 28 '22 at 08:25
7

If you must implement your own strdup function, at least rely on the rest of string.h for as much as you can. Many of these functions are optimized, and faster than you can write them yourself. For example, strlen is probably using SSE instructions, while your manual search for the null-terminator byte is not. It's also less code, which is always better. In short, I would suggest this:

#include <stdlib.h>
#include <string.h>

char *ft_strdup(const char *s1)
{
  char *str;
  size_t size = strlen(s1) + 1;

  str = malloc(size);
  if (str) {
    memcpy(str, s1, size);
  }
  return str;
}
Bert Regelink
  • 2,696
  • 23
  • 17
Enno
  • 1,736
  • 17
  • 32