Duplicate function -> exited, segmentation fault ç_ç

Question

I'm programming in C, I know why when I start my program, the terminal show me this error, but I don't know how to fix it (I have read many question about this, but no I can't solve this problem) :

My function is :

char * String_dup(char const string[]) {
  size_t size = strlen(string);
  char * copy = malloc(size * copy[0]);
  assert(copy != NULL);
  strcpy(copy, string);
  return copy;
}

it consist to duplicate my string[].

And this is my test :

void StringTest_dup(void) {
  char string[] = "voiture";
  assert(strcmp(string, String_dup(string)));
}

Thank you for your help.

you are taking the size of a static value at compile time. I would read up on strings - https://www.tutorialspoint.com/cprogramming/c_strings.htm — OldProgrammer, May 11 '21 at 00:16
`char * copy = malloc(size * copy[0]);` <- problematic line. You probably meant to use `sizeof(copy[0])`? But `sizeof(char)` is always 1. You're also not accouting for the nul terminator. So use `char * copy = malloc(size + 1);` — P.P, May 11 '21 at 00:17
You read uninitialized value `copy[0]`, this causes undefined behaviour — M.M, May 11 '21 at 00:18
Well, I don't understand very well the malloc thing, but I just want to allocate enough memory for copying my string[], this is an exam question, and we have to work on the "dynamic allocation". — Alonso, May 11 '21 at 00:24
Btw i replace the line by ```char * copy = malloc(size + 1);``` but my test fail now — Alonso, May 11 '21 at 00:25
It's going to fail because `strcmp` returns 0 if the strings are equal. That is, your code is doing `assert(0)`. Since 0 is the same thing as `false` ... — Daniel Walker, May 11 '21 at 00:27
One more important thing: your test `malloc`s memory (via `String_dup`) but never `free`s the result. You're leaking memory. — Daniel Walker, May 11 '21 at 00:29
But is that so bad if I never free this memory ?? I mean, do that slow my program or computer ? — Alonso, May 11 '21 at 00:32
@alonso00235: [What REALLY happens when you don't free after malloc?](https://stackoverflow.com/q/654754/634919) — Nate Eldredge, May 11 '21 at 01:20
`malloc` returns null or the new pointer. It is a programming error when you `assert(copy)`. The very minimal you could do is `if(!copy) exit()` or, better, `if(!copy) return 0` and document this behaviour. However, you would be justified in `assert(string)`. This is an error that is hard to recreate because it almost never happens. — Neil, May 11 '21 at 03:27

fullnitrous · Accepted Answer · 2021-05-11T01:21:35.697

I re-wrote it a bit and it seems to work without a problem. One obvious misundertanding you have is that strcmp returns 1 when they match which is wrong. If the two inputs to strcmp are the same then it returns 0, thus !strcmp(...).

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

char* String_dup(const char* string) {
        size_t size = strlen(string);
        char* copy = malloc(size * sizeof(char) + 1);
        assert(copy != NULL);
        strcpy(copy, string);
        return copy;
}

int main(void) {
        const char* string = "test";
        char* copy = String_dup(string);
        assert(!strcmp(string, copy));
        printf("%s\n", string);
        printf("%s\n", copy);
        free(copy);
        return 0;
}

Instead of a generic `sizeof (char)` it is more professional to reference the target variable, like `sizeof *copy` or `sizeof copy[0]`. This way any change in the data type is automatically taken into account. -- And I would express the assertion like `strcmp(string, copy) == 0` because this is the value I expect. Using the implicit interpretation as boolean can be misleading. — the busybee, May 11 '21 at 10:00

Duplicate function -> exited, segmentation fault ç_ç

1 Answers1