0

I was trying out linked lists and for some reason it isnt doing what it is supposed to do. When I enter the quantity after choosing 1 it is all good until the node is add to the existing list, after which the quantity becomes a weird string of numbers. And also when ever i try adding more than one node to the donate list the program crashes.

EDIT: The above problem is solved but there is another problem which I forgot to mention It is when I am trying to print the list out, nothing gets printed. This happens when I choose 4.

EDIT2: The print function is only printing out the first node nothing after that. Please help.

Here's the code.

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

typedef struct donation{
    char name[50];
    int quant;
    struct donation* next;
}donate;


donate* addItem(donate *mylist,donate *temp){
    donate *front=(donate*)malloc(sizeof(donate*));

    if(mylist==NULL)
    return temp;

    front=mylist;
    while(mylist->next!=NULL)
        mylist=mylist->next;
    mylist->next=temp;

    return front;
}    
void print(donate* donList){

    printf("Printing the Donations Table\n\n");
    if(donList!=NULL){
        while(donList->next!=NULL){
            printf("%s %d\n",donList->name,donList->quant);
            donList=donList->next;
        }
    }
}

main(){

    donate *list=NULL;

    while(1){
        int choice;
        printf("1. Add a donation\n);
        printf("Enter your choice: ");
        scanf("%d",&choice);

        if(choice==1){
            donate* temp=(donate*)malloc(sizeof(donate*));
            printf("\nEnter inventory type: ");
            scanf("%s",temp->name);
            printf("Enter the amount: ");
            scanf("%d",&temp->quant);
            temp->next=NULL;
            list=addItem(list,temp);
            printf("\nDonation Added!\n");
            printf("%s %d\n",list->name,list->quant);
        }
    else if(choice==4){
        print(list);
    }
}

    system("pause");
    return 0;
}

Thanks!

Improve this question
1
  • 1
    You should consider reducing the code in your question to the minimum needed to demonstrate your problem. For example, you can lose the printing and the command-line UI. Instead, make main() do the sequence of steps that lead to gibberish output or a crash that you described in prose. Commented Sep 24, 2012 at 1:24

4 Answers 4

Reset to default
2

One problem is that you are mallocing space for a donate pointer. You need to allocate space for the struct itself.

donate* temp=(donate*)malloc(sizeof(donate*));

should be

donate* temp= malloc(sizeof(donate));

Since you are doing a malloc, prior to adding an item, I think addItem just needs to be:

donate* addItem(donate *mylist,donate *temp)
{
    if (mylist != NULL)
       temp->next = mylist;

    return temp;
}

It looks like you would not print a 1 item list:

   printf("Printing the Donations Table\n\n");
    if(donList!=NULL){
        printf("Not NULL!!!!\n");
        while(donList->next!=NULL){
            printf("%s %d\n",donList->name,donList->quant);
            donList=donList->next;
        }
    }

I think it should be:

printf("Printing the Donations Table\n\n");
if (donList!=NULL)
{
    printf("Not NULL!!!!\n");
    do 
    {
        printf("%s %d\n",donList->name,donList->quant)
        donList=donList->next;
    }
    while(donList != NULL);
}
Improve this answer
Sign up to request clarification or add additional context in comments.

7 Comments

The same problem is also in addItem(). That allocation will leak memory if mylist == NULL. It also doesn't do anything useful because front is reassigned to mylist immediately after that.
Yes I fixed everything regarding the pointer. And I updated the question. The other problem is regrding printing out the list.
If I remove the front from addItem then how will the program know Which is the first node?
@Anj17 That's not what I meant. I meant that allocating a new front node isn't necessary if you're either returning the original value of mylist or temp. You can do donate *front = mylist; without allocating memory for this pointer if it's going to point to existing data.
Thank fixed it! Thank you so much. But can you explain why the do-while loop worked but not the while loop?
|
1

Try running your program linked to efence or with valgrind. Both will tell you when and where things start to go bad.

Improve this answer

5 Comments

click on them, i linked them to their respective manual pages.
@GungFoo Somehow I'm not sure a man page is the appropriate document to point in an answer to someone writing a Data Structures 101 assignment -ish program.
I did and it seems like those require linux and I am running windows.
why not? the man pages of said programs describe how to use them to find the error.
cygwin allows you to run linux command line programs on windows too
1

There are two issue that I see. First is the issue pointed out by Scooter. Second is you have a memory leak in the first line of addItem().

Edit To answer your second question, you will need to fix the build error; you reference reqList in main() but never declare it.

Here is a corrected version of the code:

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

typedef struct donation{
    char name[50];
    int quant;
    struct donation* next;
}donate;


donate* addItem(donate *mylist,donate *temp){
    if(mylist==NULL)
    return temp;

    donate *front=mylist;
    while(mylist->next!=NULL)
        mylist=mylist->next;
    mylist->next=temp;

    return front;
}    

main(){

    donate *list=NULL;

    while(1){
        int choice;
        printf("1. Add a donation\n);
        printf("Enter your choice: ");
        scanf("%d",&choice);

        if(choice==1){
            donate* temp=(donate*)malloc(sizeof(donate));
            printf("\nEnter inventory type: ");
            scanf("%s",temp->name);
            printf("Enter the amount: ");
            scanf("%d",&temp->quant);
            temp->next=NULL;
            list=addItem(list,temp);
            printf("\nDonation Added!\n");
            printf("%s %d\n",list->name,list->quant);
        }
    }
    system("pause");
    return 0;
}
Improve this answer

2 Comments

sorry about that reqList is not part of code so that is not the problem. What is actually happening is that it is only printing out the first node. Nothing after that.
@Anj17 at a guess I'd say you are setting list to the wrong value in main() somewhere. I'd recommend looking carefully at all of your assignments to list. With that said I don't see the error in the code present in the question.
0

just make correction here donate *front=(donate*)malloc(sizeof(donate*))
to

donate *front=(donate*)malloc(sizeof(donate))
Improve this answer

1 Comment

Best practice is donate* front = malloc(sizeof *front); ... the first * should go with the type, not the variable, the cast should be avoided because it isn't needed, is wrong if the type changes, and can hide bugs, and sizeof should work off the variable not the type so that it is independent of the type.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.