2

so I am having weird problems trying to create a node with this struct:

struct node {
    char *value ;
    unsigned int count ;
    struct node *next ;
} ;

here's what i've got

struct node *make_node(char *value) {
    struct node *np = NULL;
    *np = (*np)malloc(sizeof(*np));
    char* copy = (char*)malloc(sizeof(strlen(value)+1));
    strcpy(copy, *value);
    *np -> *value = copy;
    *np -> count = 1;
    *np -> next = null; 
    return np ;
}

the string part is throwing me i think. I'm getting a bunch of incompatable pointer types.

--EDIT-- Answered, thank you all for helpin me

Remo.D
  • 16,122
  • 6
  • 43
  • 74
user3516302
  • 81
  • 1
  • 1
  • 9
  • 3
    The language syntax violations in this alone warrant referencing you to [*any* decent **book on the C Programming Language**](http://stackoverflow.com/questions/562303/the-definitive-c-book-guide-and-list). – WhozCraig May 21 '14 at 12:10

3 Answers3

4

The allocation line should read:

np = malloc(sizeof *np);

Note:

  1. No asterisk before np on the left hand side, you don't want to dereference the pointer, you want to assign to it.
  2. No cast of malloc()'s return value.

Then you must have:

np->value = copy;

Remember that a->b (for a struct pointer a) means (*a).b.

Finally, it's spelled NULL.

Assuming you have strdup(), your function can be written like this:

struct node * make_node(const char *value) {
    struct node *np = malloc(sizeof *np);
    if(np != NULL)
    {
        np->value = strdup(value);
        if(np->value != NULL)
        {
            np->count = 1;
            np->next = NULL;
            return np;
        }
        free(np);
    }
    return NULL;
}
Community
  • 1
  • 1
unwind
  • 391,730
  • 64
  • 469
  • 606
3
struct node *make_node(char *value) {

Your function does not modify value, so it is a good habit to declare it const.

struct node *make_node(char const *value) {

*np = (*np)malloc(sizeof(*np));

Don't dereference pointer on assignment. Also casting malloc is useless in C, and can hide bugs.

np = malloc(sizeof(*np));

Note that it is highly recommended that you check result of malloc for NULL. Memory allocation can fail.


char* copy = (char*)malloc(sizeof(strlen(value)+1));

You are correctly calculating size of string here strlen(value)+1, but after that the sizeof is incorrect. It will only return the size of int.

char* copy = malloc(strlen(value)+1);

strcpy(copy, *value);

strcpy takes (char *, char const *) not (char *, char).

strcpy(copy, value);

*np -> *value = copy;
*np -> count = 1;

Again, dereferencing is not what you want. Only assign the pointer value (address).

np->value = copy;
np->count = 1;

*np -> next = null; 

You probably meant NULL here.

np->next = NULL; 
user694733
  • 15,208
  • 2
  • 42
  • 68
1
strcpy(copy, *value);

should be

strcpy(copy, value);

the strcpy function takes a char pointer as the second argument, source address.

inherithandle
  • 2,614
  • 4
  • 31
  • 53