linked list char pointer scanf input

Question

I'm attempting to input multiple times to a char pointer in linked list using scanf. but every time I enter new input the name changes in all the fields.

here is my linked list:

struct node {
struct node *next;
int level;
char *name;
};

here is my main function:

struct node *root = NULL;
while (1) {
    char arrays[12];
    char *n;
    n = arrays;
    int i = NULL;
    printf("Enter level: ");
    scanf("%i", &i);
    printf("\nEnter name: ");
    scanf("%s", arrays);
    insert(&root, i, n, compare);
    display(root);
    }

insert function:

void insert(struct node **head, const int level, char *name, int(*cmp)(struct node *l, struct node *r))
{
    struct node *new;
    new = malloc(sizeof *new);
    new->level = level;
    new->name = name;

    /* Find the insertion point */
    for (; *head != NULL; head = &(*head)->next)
    {
        if ((*head)->level > level || (*head)->level == level && cmp(*head, new) > 0) { break; }
    }
    new->next = *head;
    *head = new;
}

basically if I input:

input:        |   expected output:    |    actual output:
1     smith   |   1     john          |    1     alice
1     john    |   1     smith         |    1     alice
3     malek   |   2     alice         |    2     alice
2     alice   |   3     malek         |    3     alice

note: the functions are working as expected when I'm entering data manually without scanf eg:

insert(&root, 1, "Abbas", compare);
insert(&root, 1, "Calbass", compare);

Forgive me for asking, but I see no way for the infinite loop in your `main()` method to ever end. Also, you should show us the code for the `insert` function. — Tim Biegeleisen, Sep 16 '16 at 13:38
@TimBiegeleisen you are correct but the infinite loop was only for debugging purposes. — Poorya, Sep 16 '16 at 13:44
`scanf("%s", arrays);` is a terrible thing to do. At the very least, change it to `scanf("%11s", arrays);`. Ideally, you want to build the format string based on `sizeof arrays` — Happy Green Kid Naps, Sep 16 '16 at 14:03

Support Ukraine · Accepted Answer · 2016-09-16T13:58:45.610

This line:

new->name = name;

just change the value of pointer - it does not copy a string. So all elements in the linked list will point to arrays. So changing the contents of arrays will make it look as if all elements in the list was changed (but they wasn't).

You probably need:

strcpy(new->name, name);

and then you need to malloc memory for the string as well.

Something like:

new = malloc(sizeof *new);
new->level = level;
new->name = malloc(12 * sizeof(char));  // Memory for the string
strcpy(new->name, name);                // Copy the input string

BTW:

Change

    insert(&root, i, n, compare);

to

    insert(&root, i, arrays, compare);

and remove the nvariable. The functionality is the same but the coder is easier to read and understand.

score 0 · Answer 2 · answered Sep 16 '16 at 13:47

Look like you're inserting a pointer to arrays into the list. When you write:

insert(&root, 1, "Abbas", compare);

it's work because nothing modify the string literal "Abbas", but arrays's content is overwritten every time the scanf("%s", arrays); is executed. Consider changing char* name to char name[12] and reading input directly into the node.

linked list char pointer scanf input

2 Answers2