C linked list push function, just checking if this is correct

Question

So I'm just wondering if this push function, which pushes a new value to the top a linked list stack is correct

void push(node** hd, int v){
node temp;
temp = (node*)malloc(sizeof(node));
temp -> val = v;
temp -> next = *hd;
*hd = temp;

}

Thanks in advance!

Also I'm wondering how I would make a pop function, to pop the most recently pushed value off of the stack.

The struct code looks like this by the way...

typedef struct nodeStruct
{
int val;
struct nodeStruct* next;
}node;

typedef node* list;

It definitely looks like it has problems. SUGGESTION: why not compile and run it under the debugger? — paulsm4
– paulsm4, Commented Nov 5, 2012 at 23:16
I don't know how to use my gdb debugger, as I am short on time to finish this I'll definitely learn it for the future.. :\ If the problem is obvious I would like to just know. This is my first time using Linked Lists, and I'm not so great with pointers in just C. — Man Person
– Man Person, Commented Nov 5, 2012 at 23:23
Your edit actually made things worse. Please reread the answer by @William Morris. (and I agree: typedeffing pointers only increases confusion) — wildplasser
– wildplasser, Commented Nov 6, 2012 at 1:05

William Morris · Accepted Answer · 2012-11-06 00:42:35Z

2

Dont define pointer types. They are confusing. Define a normal type and dereference it explicitly. Here is what you were trying to do without the pointer types.

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

typedef struct nodeStruct
{
    int val;
    struct nodeStruct *next;
} node;

static void push(node **head, int v)
{
    node *temp = malloc(sizeof(node));
    temp->val = v;
    temp->next = *head;
    *head = temp;
}

int main(int argc, char **argv)
{
    (void) argv;
    (void) argc;
    node *list = NULL;

    for (int i=0; i<10; ++i) {
        push(&list, i);
    }
    for(node *l = list; l != NULL; l = l->next) {
        printf("%d ", l->val);
    }
    printf("\n");
    return 0;
}

Note that you should check for failure of malloc. In other words, malloc can return NULL, which should be handled - left to you.

edited Nov 6, 2012 at 0:42

answered Nov 5, 2012 at 23:38

William Morris

3,7142 gold badges26 silver badges25 bronze badges

Sign up to request clarification or add additional context in comments.

6 Comments

Man Person Over a year ago

For the malloc wasn't I doing node * temp = (*node)malloc(sizeof(node)); ?

Man Person Over a year ago

is the problem that it should be node temp instead of list temp, and then remove the (list) in front of malloc?

William Morris Over a year ago

The problem stemmed from your use of a typedef'd pointer (typedef node* list;). Just don't do it.

Man Person Over a year ago

Sorry I'm just a bit confused on which lists in the code I should just replace with node, and which I should replace with node*.

Man Person Over a year ago

I did, but then I'm left with a bunch of lists, do I change them to node* or node?

|

Collectives™ on Stack Overflow

C linked list push function, just checking if this is correct

1 Answer 1

6 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

6 Comments

Your Answer

Sign up or log in

Post as a guest

Related