problem with strcpy() when trying to copy a string to a string

Question

Recently started to do some c programming again and are currently having issues with an assignment. The following function are supposed to initialize a string by allocating memory for it, it is also the first assignment that I'm required to use Assert(). I have to write the funcion that dstring_initialize soo that the following works:

    DString str1, str2;

    str1 = dstring_initialize("");
    dstring_delete(&str1);
    assert(str1 == NULL);

It is probbly something basic that I have messed up and this is what the function looks like currently:

DString dstring_initialize(const char* str)
{
    assert(str != NULL);

    char* str1;

    str1 = (char*)malloc(sizeof(char));
    
    strcpy(str1, str);

    assert(str1 == str);

    return str1;
}

The error message I get is that the last Assert, Assert(str1 == str) fails and I've been trying different things but cant figure out what I've done wrong.

Welcome to SO. Please provide a [MCVE](https://stackoverflow.com/help/mcve) that shows your problem. Your snippet cannot be compiled. What is `DString`? — Gerhardh, Apr 26 '21 at 09:49
`assert(str1 == str);` is probably not what you want to do. To compare if two strings are equal, you have to use `strcmp`. That assertion checks if the addresses held by `str1` and `str` are the same, which will never be true. — mediocrevegetable1, Apr 26 '21 at 09:50
You should at least care about the length of your input string. If you only allocate memory for 1 char, you can never store anything but `""`. — Gerhardh, Apr 26 '21 at 09:50
Also, your return type is `DString` but you return a `char *`. Unless `DString` is a `typedef` of `char *`, that's bound to cause some issue. — mediocrevegetable1, Apr 26 '21 at 09:53

score 0 · Accepted Answer · answered Apr 26 '21 at 10:06

You have several fundamental misconceptions. First of all, there is no string class in C. There are just character arrays that could be used as strings if you append a null terminator \0 at the end of the array.

DString Hiding pointers char* behind typedef is incredibly bad practice. You'll only end up fooling yourself that you have some manner of string class when you don't. Get rid of this evil typedef.
str1 = (char*)malloc(sizeof(char)); This only allocates a single character, not very helpful.
strcpy(str1, str); This will copy until strcpy finds the null terminator in str. Which it does immediately since you passed "". So it will actually not copy anything other than the null terminator. It's essentially the same thing as str1[0]='\0';.
assert(str1 == str); compares addresses not contents. str1 and str are different objects and they will never have the same address, so this assert will always fail. To compare string contents, use strcmp().

From here on I'd recommend to re-visit your C book and study arrays, pointers and strings, in that order.

score 0 · Answer 2 · answered Apr 26 '21 at 10:08

0

    assert(str1 == str);

On this line, you compare two pointers. Add this line before your assert:

        printf("%p\n", str1); printf("%p\n", str); printf("%d\n", str1 == str);

If you want to compare two string, you could use strcmp for exemple.

But this is not very secure, you could check the man of strcmp and strcpy for understand why.

answered Apr 26 '21 at 10:08

d0m00re

113
1
8

2

`strcmp` and `strcpy` are perfectly safe, as long as you use them on sanitized data and not directly on user input or command line arguments etc. – Lundin Apr 26 '21 at 10:19

Bananenkönig · Answer 3 · 2021-04-26T10:21:11.420

0

First I have to mention, that when using C-Strings you normally add a lenght information. If not, you have to make sure that your strings are all Null-Terminated.

Second, this line str1 = (char*)malloc(sizeof(char)); makes no sense, because you are allocating only one character.

Third, you are comparing the pointer you got and the invalid pointer i just mentioned and not the strings themself.

edited Apr 26 '21 at 10:21

answered Apr 26 '21 at 10:11

Bananenkönig

547
4
12

1

"then you cast it to a character pointer, which has 4 or 8 byte (depends on whether you are using 32 or 64 bit). So the pointer of str1 MUST be invalid" Uh, no they cast the `void` pointer returned from malloc, pointing to that single character, from `void*` into a `char*`. How large the pointer type is on the given system is irrelevant. – Lundin Apr 26 '21 at 10:17
You are right there, i changed my answer. – Bananenkönig Apr 26 '21 at 10:21

problem with strcpy() when trying to copy a string to a string

3 Answers3