How to initialize a string into a linked list struct in C

Question

I have a structure Player, which serves as a linked list.

I'm trying to initialize the 'name var' portion of Player. I'm receiving an error: invalid conversion from 'void*' to 'char*'.

I don't understand why this is happening since i've malloc memory and then just want to copy the string over.

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

struct Player
{
   char *name;
    int jersey;
    int points;
    struct Player* next;
};




int main()
{
    struct Player *head, *ptr;

    head = (struct Player*)malloc(sizeof(struct Player));

    char playerName[] = "Hurley";
    head->name=malloc(strlen(playerName)+1);

    strncpy(head->name, "Hurley");
    head->jersey = 11;
    head->points = 15;
    head->next = NULL;

    ptr = (struct Player*)malloc(sizeof(struct Player));

    char playerName2[] = "Hill";
    ptr->name=malloc(strlen(playerName2)+1);
    strncpy(ptr->name, "Hill");


    ptr->jersey =33;
    ptr->points = 17;
    ptr->next = NULL;

    head->next=ptr;
    printf("head name: %s, next name: %s\n", head->name, ptr->name);

    free(head);
    free(ptr);

}

[Please see this discussion on why not to cast the return value of `malloc()` and family in `C`.](http://stackoverflow.com/q/605845/2173917). — Sourav Ghosh, Feb 05 '16 at 16:50
Nevermind, I fixed it! I changed it to strcpy and cast my malloc to (char*) — starter1011, Feb 05 '16 at 16:51
You're using a C++ compiler to compile C code. In C++, you have to cast the return value from `malloc()` to the target type; in C, you don't have to do it. I suspect you're using MSVC to do the compiling. Note that you have more calls to `malloc()` than you have calls to `free()`, so you are leaking memory. In this program, it doesn't matter much. In general, it can be a severe problem. — Jonathan Leffler, Feb 05 '16 at 16:51
@iharob - Honest curiosity, why would you recommend *against* strncpy? Isn't that a fantastic way to end up with a buffer overflow? If an attacker could modify the string before it's copied but after the buffer has been allocated, you're going to overflow. — antiduh, Feb 05 '16 at 16:57
I am not recommending against it, I am just pointing out that the OP used it wrong, I see that without any detail my comment seems to imply that, but I would never use `strcpy` in this situation because the length of the source string is known anyway and the target has been `malloc()`, so `memcpy()`?. — Iharob Al Asimi, Feb 05 '16 at 17:01
@starter1011 Every time you `malloc()` and check for `NULL` and use the returned pointer (*if it wasn't `NULL`*), when you are done and no longer need it you MUST `free()` the pointer. — Iharob Al Asimi, Feb 05 '16 at 17:02
@iharob OP is using printf in the same scope, so might as well use snprintf. — 2501, Feb 05 '16 at 17:04
@iharob I don't really follow, I don't understand where I check for null — starter1011, Feb 05 '16 at 17:05
@starter1011 You don't, but you should be, i think that is what iharob is trying to say. You have no guarantee that `malloc` will succeed so you should always be checking the result of `malloc`. — Fantastic Mr Fox, Feb 05 '16 at 17:10
`strncpy(head->name, "Hurley");` takes 3 arguments, not 2. Check the warning settings of your compiler, which should have caught this. — chux - Reinstate Monica, Feb 05 '16 at 18:05

score 0 · Accepted Answer · edited May 23 '17 at 10:28

The real problem and the only possible explanation is that you are compiling with a c++ compiler. In c++ there is no implicit conversion from void * to other pointer types unlike in c where there is.

It is possible that you have named your file something.cpp which causes the compiler to think this is c++ code and compile it as such, leading to the error you mention in your post.

Other recommendations, are

If you are going to copy a given number of bytes for which you have allocated a very precise ammount of space, do it like this
```
const char *playerName = "Hurley";
size_t length = strlen(playerName);

player->name = malloc(length + 1);
if (player->name != NULL)
    memcpy(player->name, playerName, length + 1);
```
and even better than this, you can use strdup() instead. Also have to check for NULL before using the player->name pointer because it does return NULL when a problem happens like malloc() too.
If you are writing c code, it's recommended to avoid casting void * read this for more information. Some c programmers might disagree but most of them think that it's better not to cast.
Don't repeat yourself. Or the very well known DRY principle, in your code it seems appropriate to create a function create_player() to fill an instance of a struct, you are repeating code as it is and that is not good.

Okay. this all makes sense. thank you for the code suggestions and the theory. — starter1011, Feb 05 '16 at 17:13

How to initialize a string into a linked list struct in C

1 Answers1