Linked Lists!

[horrificmonster]

Hello!
Having a problem with linked lists. Working on a problem in this book I'm reading.

Problem is as follows:

Write a program that adds elements to a linked list in sorted order, rather than at the beginning.

Here's my code:

#include <iostream>
#include <string>

using namespace std;

struct sortedno
{
    int number;
    sortedno* nextno;
};

void getinput(int& argument);
sortedno* addno(int argument, sortedno* args);
void displaynos(sortedno* args);

int main()
{
    sortedno* args;
    int argument=1;
    while(argument>0)
    {
    getinput(argument);
    args = addno(argument, args);
    }
    displaynos(args);

}

void getinput(int& argument)
{
    //Prompt user for input, then store in referenced variable.
    cout<<"Please enter a number into the array:\n";
    cin>>argument;
}

sortedno* addno(int argument, sortedno* args)
{
    //Create new instance of sortedno, then set the number to the users input.
    sortedno* temparg = new sortedno;
    temparg->number = argument;
    temparg->nextno = NULL;
    sortedno* prevarg = NULL;
    sortedno* firstarg = args;
    //Cycle through all instances of the args linked list.
    while(args->nextno != NULL)
    {
        //If this is the first addition to the list, then point the linked list to the temp arg.
        if(args==NULL)
        {
            return temparg;
        }
        //After we check for null value, then look to see if the current arg node has a number greater than the temp arg.
        else if(args->number > temparg->number)
        {
            //Check to see if we are inserting before the first element in the linked list. If so, then return the temparg as the header for the linked list.
            if(args == firstarg)
            {
                temparg->nextno = args;
                return temparg;
            }
            else
            {
                //If we find that this node needs to be inserted, set the pointers to include this new instance of memory in the linked list.
                prevarg->nextno = temparg;
                temparg->nextno = args;
                return firstarg;
            }
        }
        //If we make it to the end of the list and haven't inserted this value yet, add to end of list.
        else if(args->nextno==NULL)
        {
            prevarg->nextno = temparg;
            temparg->nextno = NULL;
            return firstarg;
        }
        prevarg = args;
        args = args->nextno;
    }



}
void displaynos(sortedno* args)
{
    while(args->nextno != NULL)
    {
        cout<<args->number<<endl;

        args = args->nextno;
    }
}

So in essence, I'm creating a struct that holds the user input and a pointer. I use 'args' as the head of the list, and perform a while loop to add the values to the linked list. When I run the program, it seems to take the values fine. But I get some segmentation faults when I try to display the values. Something is wrong with my displaynos function, but I can't for the life of me figure it out. I definitely want to completely understand linked lists before I move on to recursion and binary trees. Wondering if someone can 'point' me in the right direction.

[Yay295]

I can't even run this on my computer so it might take a while, but first change main to this:

int main ( )
{
	sortedno * args = nullptr; // you need to set this to something. nullptr is like null, but specific for pointers
	int argument;

	while ( true )
	{
		getinput ( argument );
		if ( argument > 0 ) args = addno ( argument, args ); // I'm assuming we don't want to add negative numbers, so check for that
		else break;
	}

	displaynos ( args );
}

and since you use the new operator, you should really have a function to delete your list.

[Yay295]

here's a delete function that should work:

void deletelist ( sortedno * headptr )
{
	sortedno * todelete;

	while ( headptr != nullptr )
	{
		todelete = headptr;
		headptr = headptr->nextno;
		delete todelete;
	}
}

[long double main]

I think the main problem is that args isn't initialized to anything on line 18, so it contains a garbage value (not necessarily NULL).

Then, you pass it to the addno function, where you try to access args->nextno on line 45.

I think you should be looping on while (args != NULL) instead of while (args->nextno != NULL) (same with your displaynos function), and initialize args to a null pointer on line 18.

[Cody0023]

This linked list would be better made in a class. Variable names are better labeled as head for the first node, and the structures which act as nodes are better labeled as just node.

[Lowest0ne]

The segfault comes from line 45, which ( coupled with Yay295's post ), has you dereferencing an invalid pointer.

So to start, do what Yay295 says, and then change line 45 to:
while ( firstarg )

Which I'm not sure is going to solve all of your problems, but will at least move you along.

[horrificmonster]

Thanks for the help! I don't know why, but I was using args->nextno for my while loop (great catch, thanks!). That helped with the segmentation. I still have to make some adjustment to my addno function, but other than that it works! I'll also have to make a point to create a delete function every time I use new! The header and node comment makes sense, I can see how that would make it easier to read.

Thanks all!