Himanshu Sourav
Himanshu Sourav

Reputation: 710

storing and printing string in void pointer

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;
}

Upvotes: 4

Views: 4678

Answers (5)

Himanshu Sourav
Himanshu Sourav

Reputation: 710

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;
}

Upvotes: 1

RoadRunner
RoadRunner

Reputation: 26315

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.

Upvotes: 1

Tom Taylor
Tom Taylor

Reputation: 3540

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.

Upvotes: -1

Sumit Gemini
Sumit Gemini

Reputation: 1836

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;

Upvotes: 1

Sourav Ghosh
Sourav Ghosh

Reputation: 134326

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.

Upvotes: 4

Related Questions