unable to insert element into linked list

Question

I am trying to write a program that finds all the ")" in an expression and puts them in a linked list, always adding at the beginning of the list. The problem is that when I try to place a new element into the list, the program stops working.

With a sample user input 865)987:

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

typedef struct element {
    char data;
    struct element *next;
} ELEMENT;


int main(void)
{
   ELEMENT *first = NULL;
   ELEMENT *new_a;

   char input[30];
   int x=0;

   printf("Input expression: ");
   scanf("%s", &input);

   while(input[x]!='\0'){
       if (input[x]==')'){
           printf("%c", input[x]);        //This works just fine.
           new_a->data = input[x];        //Here, the program stops working.
           new_a->next = first;
           first = new_a;
       }
       x++;
    }
}

What am I doing wrong?

score 4 · Answer 1 · edited May 23 '17 at 11:51

new_a->data

is equivalent to

(*new_a).data

As you can see, new_a is attempted to be dereferenced. The problem is that new_a is uninitialized, so any subsequent attempt to dereference it is undefined behavior (in shape of, e.g., a segmentation fault).

In order to fix this, you need to allocate memory for new_a:

Allocate space on the stack. This will only work if the linked list is exclusively used in main because local variables' scope only embraces the beginning and end of a function.
Do it like this:
```
ELEMENT new_a;

...

new_a.data = input[x];
new_a.next = first;
first = &new_a;
```
Use malloc. This is usually used for linked lists and is applicable for a linked list existing till the very termination of your program because it's scope-independent:
```
ELEMENT* new_a = malloc(sizeof(ELEMENT));
```
Don't forget to free afterwards!

Notes:

Passing input to scanf suffices; no need for &input.
Your scanf is vulnerable to buffer overflows. Use
```
scanf("%s29", input);
```
instead. Read this also.

Thank you very much, now it seems to be working! Just a minor problem: There is a new warning on the line with "first = &new_a;", "assignment from incompatible pointer type". Do you happen to know why that is? — DerekT, Nov 22 '15 at 11:11
@DerekT **Either** allocate on the stack with `ELEMENT new_a;` **(note the lack of the asterisk!)** and use `first = &new_a;` **or** allocate on the free store with `malloc` and use `first = new_a;`. Or is it something else? If your issue isn't fixed, *add the error message*, please. — cadaniluk, Nov 22 '15 at 11:12

score 0 · Answer 2 · answered Nov 22 '15 at 10:58

0

You need to allocate memory for new_a:

new_a = malloc(sizeof(ELEMENT));

answered Nov 22 '15 at 10:58

Programmer dude

167
1
5
23

Anton Todua · Answer 3 · 2015-11-22T11:11:00.550

0

As previously answered, the correct code is:

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

typedef struct element {
    char data;
    struct element *next;
} ELEMENT;

int main(void)
{
    ELEMENT *first = NULL;  
    char input[30];
    int x=0;

    printf("Input expression: ");
    scanf("%s", &input);

    while(input[x]!='\0'){
        if (input[x]==')'){
            ELEMENT *new_a = (ELEMENT*)malloc(sizeof(ELEMENT));
            printf("%c", input[x]);
            new_a->data = input[x];
            new_a->next = first;
            first = new_a;
        }
        x++;
    }
}

edited Nov 22 '15 at 11:11

answered Nov 22 '15 at 11:08

Anton Todua

667
4
15

scanf("%s", &input); should be scanf("%s", input);, dont need to cast malloc return value. and for consistency should have return 0 in main – amdixon Nov 23 '15 at 11:32
@amdixon Of cause, you're right! But I just copy athor code and fix error of adding nodes to list. However, I think that cast return value of malloc better then don't do it. – Anton Todua Nov 23 '15 at 18:12

unable to insert element into linked list

3 Answers3