Assigning a value to an integer in a C linked list

Question

Hello all. I have a question regarding linked lists. I have the following structs and function for example.

struct node {
    int value;
    struct node *next;
};

struct entrynode {
    struct node *first;
    struct node *last;
    int length;
};
void addnode(struct entrynode *entry) {
    struct node *nextnode = (struct node *)malloc(sizeof(struct node));
    int temp;
    if(entry->first == NULL) {
        printf("Please enter an integer.\n");
        scanf("%d", &temp);
        nextnode->value = temp;
        nextnode->next = NULL;
        entry->first = nextnode;
        entry->last = nextnode;
        entry->length++;
    } else {
        entry->last->next = nextnode;
        printf("Please enter an integer.\n");
        scanf("%d", nextnode->value);
        nextnode->next = NULL;
        entry->last = nextnode;
        entry->length++;
    }

}

In the first part of the if statement, I store input into a temp variable and then assign that to a field in the struct. The else branch, I tried to assign it directly which did not work. How would I go about assigning it directly?

Thanks for your time.

Answer 1

Try scanf("%d", &(nextnode->value));

Answer 2

It should be the address of the value: scanf("%d", &(nextnode->value));

Generally speaking, as a comment on your code, avoid to repeat the code known as copy-paste antipattern. Re-use it at maximum. Usually if you find yourself in the situation to repeat code, create a small function.

Answer 3

    scanf("%d", nextnode->value);

You need to pass a pointer to the value member to keep scanf() happy. Fix:

    scanf("%d", &nextnode->value);

Perhaps one lesson to learn from this is to never mix up data entry code with data structure modification code.

Btw: please don't use unnecessary parentheses. You'll never learn the precedence rules if you do.

Answer 4

First, you have a bug.

One of the lines should be:

scanf("%d", &(nextnode->value));

And sorry to say this, but your code is horrible!

• Use a better name than entrynode. If it is a linked list, why don't you just call it that?

• I suggest you implement a method of the following signature:

  bool addnode(struct entrynode *entry, int value);

• The return value lets you know if the addition was successful.

• You have a lot of code duplication. Try to remove that.

• Use the above method after making a call to printf and scanf.

It makes me shudder to see printf and scanf littered within data structure insert methods and redundant copies of code littered in if then elses.

Answer 5

Also, please check the return value of malloc() calls. As well, please do not cast the return value of malloc() in the C programming language.