3
votes

I have written a linked list program which stores data member as void *. while trying to store annd print using scanf/printf functions, I am getting segmentation fault.

node definition -->

typedef struct node {
        struct node *next;
        void *data;
        }node;

main function -->

                head=(node *)malloc(sizeof(node));
                if (head==NULL){
                        printf("error in allocation of memory\n");
                        exit(EXIT_FAILURE);
                }
                tail=(node*)create(head);

create function -->

void *create(node *current)
{
        int user_choice;
        while(current){
                printf("\nEnter the data:");
                scanf("%s",current->data);
                printf("stored at %p\n",(void*)current->data);
                printf("%s",(char*)current->data);
                printf("\nType '1' to continue, '0' to exit:\n");
                scanf("%d",&user_choice);

                if(user_choice == 1){
                        current->next=(node*)malloc(sizeof(node));
                        current=current->next;
                }
                else{
                        current->next=NULL;
                }
        }
        return current;
}

can anyone tell what is the correct argument for scanf & prinf should be..?


working code after incorporating points given in answers...

void *create(node *current)
{
        node *temp;
        int user_choice;
        while(current){
                printf("\nEnter the data:");
                current->data=(char*)malloc(10*sizeof(char));
                scanf("%s",current->data);
                printf("stored at %p\n",(void*)current->data);
                printf("%s",(char*)current->data);
                printf("\nType '1' to continue, '0' to exit:\n");
                scanf("%d",&user_choice);

                if(user_choice == 1){
                        current->next=(node*)malloc(sizeof(node));
                }
                else{
                        current->next=NULL;
                        temp=current;
                }
                current=current->next;
        }
        return temp;
}
5
When the user presses 0, the function will return NULL. There are many errors. Please read a book, rethink your code, and then come back if you have problems.Paul Ogilvie
When head is null, and you print the error message (which should go to stderr, not stdout), you should do something other than continue with the next line that requires head be non-null. Your program has undefined behaviour.Toby Speight
@PaulOgilvie thanks for the sharp observation, I have modified code to rectify this problem and other errors. could you have one more look to see if any other issue is there..? posted in answer.Himanshu Sourav
I don't see any improvements. Still many errors.Paul Ogilvie
@PaulOgilvie, I had added in answer, updated question as well...Himanshu Sourav

5 Answers

3
votes

In your code,

 scanf("%s",current->data);

is attempt to make use of an unitialized pointer, it invokes undefined behavior.

You need to follow either of bellow approach,

  • make the pointer point to valid chunk of memory (using malloc() and family for dynamic allocation, for example)
  • use an array.
1
votes

You should first initialize data member of structure because

current->data  = malloc("passes size here");

For putting data you have to typecast first this data because void is not storage type. void pointer can be used to point to any data type.

Like

*(char *)(current->data) = 1;
1
votes

As others have said:

scanf("%s",current->data);

Is undefined in C. current->data needs to be pointing somewhere before you can store anything in it.

You should instead:

  1. Accept input from scanf.
  2. Store in temporary buffer.
  3. Insert into linked list
  4. print out whole linked list at the end
  5. free() linked list at the end.

I also feel that your current void *create function is doing too much, and it would be easier to split up your code into different functions, just to make it easier to handle all the pointer operations, inserting etc.

To demonstrate these points, I wrote some code a while ago which does these things, and has been modified to help you with your code. It is not the best code, but it does use these points that will help you with your code.

Here it is:

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

#define MAXSTRLEN 100

typedef struct node {
    void *data;
    struct node *next;
} node_t;

typedef struct {
    node_t *head;
    node_t *foot;
} list_t;

list_t *create_list(void);
node_t *generate_node(void);
list_t *insert_node(list_t *list, char *data);
void print_list(list_t *list);
void free_list(list_t *list);

int
main(int argc, char *argv[]) {
    list_t *list;
    char data[MAXSTRLEN];
    int user_choice;

    list = create_list();

    while (1) {
        printf("Enter the data: ");
        scanf("%s", data);

        printf("\nType '1' to continue, '0' to exit:\n");
        if (scanf("%d",&user_choice) != 1) {
            printf("Invalid input\n");
            exit(EXIT_FAILURE);
        }

        if (user_choice == 1) {
            list = insert_node(list, data);
        } else {
            list = insert_node(list, data);
            break;
        }
    }

    print_list(list);

    free_list(list);
    list = NULL;

    return 0;
}

/* inserting at foot, you can insert at the head if you wish. */
list_t
*insert_node(list_t *list, char *data) {
    node_t *newnode = generate_node();

    newnode->data = malloc(strlen(data)+1);
    strcpy(newnode->data, data);

    newnode->next = NULL;
    if (list->foot == NULL) {
        list->head = newnode;
        list->foot = newnode;
    } else {
        list->foot->next = newnode;
        list->foot = newnode;
    }
    return list;

}

node_t
*generate_node(void) {
    node_t *new = malloc(sizeof(*new));
    new->data = NULL;
    return new;
}

void
print_list(list_t *list) {
    node_t *curr = list->head;

    printf("\nlinked list data:\n");
    while(curr != NULL) {
        printf("%s\n", (char*)curr->data);
        curr = curr->next;
    }
}

list_t
*create_list(void) {
    list_t *list = malloc(sizeof(*list));

    if (list == NULL) {
        fprintf(stderr, "%s\n", "Error allocating memory");
        exit(EXIT_FAILURE);
    }

    list->head = NULL;
    list->foot = NULL;
    return list;
}

void
free_list(list_t *list) {
    node_t *curr, *prev;
    curr = list->head;
    while (curr) {
        prev = curr;
        curr = curr->next;
        free(prev);
    }
    free(list);
}

UPDATE:

Also note how I allocated memory for newnode->data?

Like this:

newnode->data = malloc(strlen(data)+1); //using buffer from scanf

This now means I can store data in this pointer, your current->data will need to do something similar.

1
votes

working code-->

void *create(node *current)
{
        node *temp;
        int user_choice;
        while(current){
                printf("\nEnter the data:");
                current->data=(char*)malloc(10*sizeof(char));
                scanf("%s",current->data);
                printf("stored at %p\n",(void*)current->data);
                printf("%s",(char*)current->data);
                printf("\nType '1' to continue, '0' to exit:\n");
                scanf("%d",&user_choice);

                if(user_choice == 1){
                        current->next=(node*)malloc(sizeof(node));
                }
                else{
                        current->next=NULL;
                        temp=current;
                }
                current=current->next;
        }
        return temp;
}
-1
votes

Please try with this

void *create(node *current)
{
        int user_choice;
        while(true){
                if(current == NULL) {
                   current = (node *)malloc(sizeof(node));
                   current->data = NULL;
                   current->next = NULL;
                }
                printf("\nEnter the data:");
                scanf("%s",current->data);
                printf("stored at %p\n", (void *)current->data);
                printf("%s",current->data);
                //printf("%s",(char*)current->data);
                printf("\nType '1' to continue, '0' to exit:\n");
                scanf("%d",&user_choice);

                if(user_choice == 1){
                        current->next=(node*)malloc(sizeof(node));
                        current=current->next;
                }
                else{
                        current->next=NULL;
                        tail = current;
                        current=current->next;
                        break;
                }
        }
        return current;
}

Note: The element has to be initialized (ie; it has to be alloted with some memory) before we are trying to make use of it.