Allocating space to a node pointer

Question

I am currently attempting to use a doubly linked list to sort some data. I am having trouble creating a new node with the given data. Below was the code given to me:

#ifndef LIST_H_
#define List_H_
#define MAX_SYMBOL_LENGTH 7
struct order {
    int id;
    char symbol[MAX_SYMBOL_LENGTH];
    char side;
    int quantity;
    double price;
};

typedef struct order* OrderPtr;
typedef struct onode* NodePtr;

struct onode {
    OrderPtr data;
    NodePtr next;
    NodePtr prev;
};

This is the code that I have written using list.h as a header. Here is the code that seemingly keeps crashing:

#include "list.h"

NodePtr newNode(OrderPtr data){

    NodePtr node = (NodePtr)malloc(sizeof(NodePtr));
    //node->data = (NodePtr)malloc(sizeof(OrderPtr));
    //*node->data = *data;
    node->data = data;//This is the one I am having problems with
    node->next = NULL;
    node->prev = NULL;
    return node;
}

It compiles fine but when I try and submit it to an online grader it says that it does not work. Here is my thought process,

create memory for NodePtr.
create memory for NodePtr->data.

and then assign the values of data passed from the function to the values in Node->Ptr. But I do not know how to allocate memory for NodePtr->data.

You don't need to cast the return value of `malloc` in a C program. — Carl Norum, Oct 18 '13 at 22:52
Also, when referring to code in-line, it helps to surround it with backticks, so \`code\` will show up as `code`. — Jonathon Reinhart, Oct 18 '13 at 22:52
You'll just have to trust me when I say typedef-ing pointer types has only one positive attribute: it avoids incorrect declarations. `NodePtr a,b` vs `Node* a,b;`. Other than that, there is no real benefit, and you'll find most professional C/C++ programmers want to *see those asterisks in-code* and not hide them in a unneeded typedef, if for nothing else than as a giant flag saying "Look! A Pointer!" Unless you're writing an abstract handle-type system that hides pointers as "handles", don't make it a habit. — WhozCraig, Oct 18 '13 at 23:17

score 2 · Accepted Answer · edited May 23 '17 at 10:25

2

NodePtr node = (NodePtr)malloc(sizeof(NodePtr));

Isn't doing what you are thinking. It's allocate space to hold a pointer same as sizeof(int*), it's 4 bytes on 32-bit machine, usually.

You need to do NodePtr node = malloc(sizeof(struct onode)); instead of. data member should be result to a malloc(sizeof(struct order));

Also, don't cast result value from a malloc() call.

edited May 23 '17 at 10:25

Community

1
1

answered Oct 18 '13 at 23:06

The Mask

17,007
37
111
185

Okay I see so I only call malloc once overall? Wouldn't you mean struct onode? Another person said to call onode not struct order. I understand it better why not to cast now though thank you for the link – JumpingRock Oct 18 '13 at 23:12
Its not `sizeof(struct order)` he needs; its the size of a `struct onode`, and like many I prefer simply using a dereference of the declared variable as the `sizeof` parameter to ensure you have the right type: I.e. `NodePtr node = malloc(sizeof(*node));`, which avoids incorrect type-sizing for malloc in the first place. – WhozCraig Oct 18 '13 at 23:13
It's `sizeof(struct onode)` really. My mistake. – The Mask Oct 18 '13 at 23:15
So i have been working for the past hour or so on getting my newNode function to work. Do you guys have any idea on how to copy the data? It seems that putting `node->data = data` does not work. I tried using `memcpy(node-data, data, sizeof(*node)` with no luck. Any ideas? – JumpingRock Oct 19 '13 at 00:33
1

You need to post the code that calls the node create, and the data pointer. Probably you have not adequately allocated the data pointer and are just using stray stack :-) – ChuckCottrill Oct 19 '13 at 00:52

score 1 · Answer 2 · answered Oct 18 '13 at 23:01

1

NodePtr is a pointer to a node and not the node itself. You're only allocating enough memory for a pointer and not all the members of the onode structure. You'll want to call malloc with sizeof(struct onode).

answered Oct 18 '13 at 23:01

greeny

96
2

How exactly do you post code in the comments? Do you know? NodePtr node = (NodePtr)malloc(sizeof(struct onode)); Is what I will implement, so just to be sure I do only call malloc once overall? – JumpingRock Oct 18 '13 at 23:08
You can use the '`' character that shares the same key as ~ below ESC. When you are typing there is a box that shows up underneath that will tell you more about other features. Yes, you only need to call malloc once. – greeny Oct 18 '13 at 23:16

Allocating space to a node pointer

2 Answers2