Adding a struct to linkedlist in c

Question

I'm trying to add Process-structs to a linked list. Their definitions are as follows.

typedef struct {
    char name[2];
    int duration;
    int priority;
    int arrival;
} Process;

typedef struct {
    Process p;
    struct LinklistNode* next;
} LinklistNode;

The function that I'm using to create the process looks like this.

Process makeProcess(char nameIn[2], int durationIn, int priorityIn, int arrivalIn){
    Process p = (Process*) malloc(sizeof(Process)); //getting an error
    p->name = nameIn;
    p->duration = durationIn;
    p->arrival = arrivalIn;
    p->priority = priorityIn;
}

I'm not sure that I'm doing that part right, and I'm also not sure that I should be returning a process or have it void, as this process "should" go into the linked list.

My code for creating a linked list node is as follows:

LinklistNode* create_linklist_node(Process pIn) {
    LinklistNode* node = (LinklistNode*) malloc(sizeof(LinklistNode));
    node->p = pIn;
    node->next = NULL;
    return node;
}

For a bit more context I'll be calling these functions in main() where I've tokenized a string from a file I'm reading from. I'm wondering the best way to make the Process struct. Right now I have this:

while(!feof(fPointer)){
    //the i counter is for the first line in the text file which I want to skip
    while ((fgets(singleLine, 1500, fPointer) != NULL) && !(i == 0)){
        char *token = strtok (singleLine, delimit);

        while(token != NULL){
            printf(" %s\n", token);
            token = strtok(NULL, delimit);



        }

    }
    i++;
}

Bit of a long question but any references or additional information is always appreciated. Let me know if you have additional questions or need more info on what I'm doing or why I'm doing something. Or if you can find an example of something similar, that would be greatly appreciated as I haven't had much luck with that so far.

Thanks

In `Process p = (Process*)malloc(...` the variable `p` is a struct, not a pointer. So it can't take the return value from `malloc`. — Weather Vane, Jul 14 '16 at 18:58
I didn't read much more but I see you use `feof`, so please read [why `feof` is usually wrong](http://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong). Just go for `while(fgets(...) != NULL)` — Weather Vane, Jul 14 '16 at 19:00
Regarding what Weather Vane said, you should declare `p` like this: `Process* p = (Process*) malloc(sizeof(Process));` — Filip Allberg, Jul 14 '16 at 19:19
Relevant resources: http://cslibrary.stanford.edu/103/LinkedListBasics.pdf http://cslibrary.stanford.edu/105/LinkedListProblems.pdf — Filip Allberg, Jul 14 '16 at 19:26

score 2 · Accepted Answer · edited Jul 14 '16 at 19:53

You seem to have some issues with pointers. In this line

Process p = (Process*)malloc(sizeof(Process)); //getting an error

what you should be doing is

Process *p = malloc(sizeof(Process));

because unlike a new in some other languages, the malloc will just return a void *, (which in pure C can be automatically converted to any other data object pointer type). That pointer stores the address of the memory allocated for your struct. Of course you will also have to return the pointer, thus changing the return type to Process*.

Continuing with your original design, you would also have to store the Process* in the list-node, and consequently pass it to your construction method (LinklistNode* create_linklist_node(Process *pIn)).

You would then have to free both the node and possibly the pointer to the contained struct, if it is no longer used anywhere else, when you destroy the node.

However, given the size of your Process struct, I would suggest something else:

Since you already have your list nodes like this:

typedef struct{
    Process p;
    struct LinklistNode* next;
}LinklistNode;

You would allocate the memory for the actual process struct inside the node in during the call to create_linklist_node. Then you can just pass in a Process struct that is on the stack and copy it into the struct in the list, which lives in the heap-memory allocated by the create-call. In that case you don't need to dynamically allocate the Process at all, and the pointer issues in the first part become irrelevant.

LinkListNode *create_linklist_node(Process proc)
{
    LinklistNode *p = malloc(sizeof *p);
    if (p == NULL)
    {
        perror("Failed to allocate new node: ");
        exit(EXIT_FAILURE);
    }
    p->p = proc;
    p->next = NULL;
    return p;
}

You would then do something like this:

Process proc = {{'a', 'b'}, 0, 0, 0};
LinklistNode *p = create_linklist_node(proc);

However, it is more common to have a method that directly creates and inserts the node into the list, for example, given:

typedef struct {
    LinklistNode *head;
} Linklist; //doing this is kinda optional, using a simple pointer would do too

you could insert at the head of the list by doing something like:

void insert (Linklist *list, Process ins) 
{
    LinklistNode *tmp = create_linklist_node(ins);
    tmp->next = list->head;
    list->head = tmp;
}

Adding a struct to linkedlist in c

1 Answers1