Dynamic memory problem

I ran your program. It does not have the problem your said. It works as you write for windows.

Hi,

Have you tried using a debugger? One can have a list of variable values to watch, and step through the program 1 line at a time, to deduce where it all goes wrong.

If you are using an IDE, it should have one built in that is pretty easy to use. Alternatively, there are command line versions, which are a bit harder.

This will save you days of frustration, and is really an essential skill to learn.

Hope all goes well :+)

Edit:

A couple of observations:

1. Your code is a mixture of C and C++ (mainly C). Try to avoid doing that if you can. I know I went through a transition phase, because I had already Learnt C a long time ago. C tends to have a different mindset: process things one at a time - use pointers everywhere. C++ has the STL with containers, algorithms and various classes to make one's life easier. Although I understand that school assignments often require one's own implementation in order to learn how things work.

2. Investigate using smart pointers, then you won't have to worry about using new or delete ever again :+)

Last edited on

Horscht (26)

The problem is you are assigning the same memory adress where the name is stored to the new passenger everytime.
Every passenger has a pointer to the variable "name" in your main().
You need to give every instance it's own space in memory where it can store the name.
Something like

case '1':
{
/*>>*/	char* newName = new char[PASSENGER_NAME];
	cout << "Name: ";
/*>>*/	cin >> newName;
	cout << "\nFlight: ";
	cin >> flight;
	cout << "\nSN: ";
	cin >> seat_num;
	cout << "\nPaid: ";
	cin >> price_paid;
/*>>*/	addPassenger(head, last, newName, flight, seat_num, price_paid);
}

The problem here is that using "new", the memory will be allocated on the heap instead of the stack, so you will need to free the memory again once you delete your passenger.
So in your removePassenger function, it should look something like this:

else if (head == last)
{
	delete[] head.name;
	delete head;
	head = NULL;
	last = NULL;
}

else 
{
	Passenger *temp = head;
	head = head -> next;
	delete[] temp.name;
	delete temp;
}

I did not test it, but at least you should now know where the problem is :)
Maybe i'm wrong, haven't done anything with archaic C style stuff in a long time. For modern code I would recommend using std::string and smart pointers, but that may be too complicated for beginners, or teachers want you to do it this way instead of the better way ;)

Why noone advice to use string instead of char*?

Thank you for your replies! It does help! My code is definitely a mixture of C and C++, for I've learnt only C++ and the class I'm taking is C and the teacher doesn't know C++, so I cannot use classes or anything beautiful that C++ provides within its language. The assignment is to do the thing using linked lists, so yeah, kinda have to do it like this.

Horscht, thank you, but it doesn't work :) Still when I display passengers list, passengers (no matter how many there are) have same names and flights, so I figured the problem could be also in this line:

printf("\nName: %s\nFlight: %s\nSeat: %d\nPaid: %f\n", current -> name, current -> flight, current -> seat_num, current -> price_paid);

Unfortunately, I do not know how to fix it. I tried dedicating new memory, but it would give me only some mumba-jumba.

I don't know how to use string and smart pointers, but that's a way to dig in, so I shall do, but for this project, I cannot use them :(

And yeah, I don't have a debugger. Stuff on linux is just compicated, lol. I'm using "Geany" right now to view and edit code. It also has option to compile and build, so I've stopped on this one.

Horscht (26)

Found a probably better solution without the new and delete stuff:
In your struct instead of having pointers, use this:

1
2

	char name[PASSENGER_NAME];
	char flight[FLIGHTS_NUM]; // or whatever max chars your flight "string" should be

This gives every instance of passenger it's own memory to store it's name.
But when assigning you can't just assign this->name = p_name, because that only assigns the adress of the variable you read the name into (char name in main()).
So you need to use strcpy() to copy the characters from name to the passenger.
Like this:

void insertAsFirst(Passenger *&head, Passenger *&last, char *p_name,
				   char *p_flight, int p_seat_num, float p_price_paid){
//...
	strcpy(temp->name, p_name);
	//temp->name = p_name; <---- Instead of THIS
	strcpy(temp->flight, p_flight);
	//temp->flight = p_flight; <---- Instead of THIS
// ...
}

And the same in addPassenger().
Everything else can stay the same like in your original code that you posted.
Working in this old style is really a pain in the ass, especially if you want to make it safe (not able to write more characters than allowed and no illegal characters etc...).

Yaaay, it works. Thank you so much! That's why I don't like pointers. One thing is that I don't completely understand them, and the other one is that using arrays is so much safer... But I'll definitely find out more about smart pointers and strings. Thanks again!

Last edited on

http://plugins.geany.org/debugger.html

Hi,

Micard wrote:
And yeah, I don't have a debugger. Stuff on linux is just compicated, lol. I'm using "Geany" right now to view and edit code. It also has option to compile and build, so I've stopped on this one.

I use Linux all the time & try to avoid using Windows. I have several IDE's on my system, they all have debuggers built in - one of the easiest ones to learn is QtCreator. It may well be worth your time to change if you can.

I just looked - Geany does have a debugger plugin:

The shell command to debug is called gdb. In a shell, type man gdb to see it's manual (help )page.

The other things to note are your compiler - clang seems to be the best - it has really nice easy to understand messages.

Also, set your compiler warnings to their maximum I routinely use -Wall -Wextra -pedantic as a minimum on clang++ or g++. It is worth reading the compiler manual, as there are some switches that are not set despite the switches mentioned.

Hope all goes well :+)

TheIdeasMan, wow, thank you! It's definitely worth to look into it!

https://www.kdevelop.org/

No worries :+D

Another IDE worth looking at is KDevelop. One can use use to develop in dozens of languages (provided you have a compiler for each one). Things like Qt and sfml et al can be integrated into it.

It is a very mature application, meaning that there has been a lot of development time & bug fixing being done.

Noone advice to use string instead of char*. Sadly.
So i'll do.
Here i change all "char*" to "string". Also i remove "printf" as slow and change it into "cout <<":

#include <cstdio>
#include <iostream>
using namespace std;

struct Passenger {
	string name;
	string flight;
	int seat_num;
	float price_paid;
	Passenger *next;
};

bool isEmpty(Passenger *head);
char menu();
void insertAsFirst(Passenger *&head, Passenger *&last, string p_name, 
					string p_flight, int p_seat_num, float p_price_paid);
void addPassenger(Passenger *&head, Passenger *&last, string p_name, 
					string p_flight, int p_seat_num, float p_price_paid);
void removePassenger(Passenger *&head, Passenger *&last);
void showlist(Passenger *curent);

bool isEmpty(Passenger *head)
{
	if (head == NULL) return true;
	else return false;
}

char menu()
{
	char choice;
	cout << "\nOption: ";
	cin >> choice;
	return choice;
}

void insertAsFirst(Passenger *&head, Passenger *&last, string p_name, 
					string p_flight, int p_seat_num, float p_price_paid)
{
	Passenger *temp = new Passenger;
	temp -> name = p_name;
	temp -> flight = p_flight;
	temp -> seat_num = p_seat_num;
	temp -> price_paid = p_price_paid;
	temp -> next = NULL;
	head = temp;
	last = temp;
}

void addPassenger(Passenger *&head, Passenger *&last, string p_name, 
					string p_flight, int p_seat_num, float p_price_paid)
{
	if(isEmpty(head))
		insertAsFirst(head, last, p_name, p_flight, p_seat_num, p_price_paid);
	else {
		Passenger *temp = new Passenger;
		temp->name = p_name;
		temp->flight = p_flight;
		temp->seat_num = p_seat_num;
		temp->price_paid = p_price_paid;	
		temp->next = NULL;
		last->next = temp;
		last = temp;
	}
}

void removePassenger(Passenger *&head, Passenger *&last)
{
	if(isEmpty(head))
		cout << "Error: The list is already empty!\n";
	else if (head == last) {
		delete head;
		head = NULL;
		last = NULL;
	} else {
		Passenger *temp = head;
		head = head -> next;
		delete temp;
	}
}

void showlist(Passenger *current)
{
	if(isEmpty(current))
		cout << "Error: Cannot display list becaue it is empty!\n";
	else {
		cout << "Passengers: \n";
		while(current != NULL)
		{
		cout << "\nName: " << current->name
			<< "\nFlight: " << current->flight
			<< "\nSeat: " << current->seat_num
			<< "\nPaid: " << current -> price_paid
			<< "\n\n"; 
		 current = current->next;
		}
	}
}

int main(int argc, char **argv)
{
	//Used for menu manipulations
	char choice;
	string name;
	string flight;
	int seat_num = 0;
	float price_paid = 0;
	
	Passenger *head = NULL;
	Passenger *last = NULL;
	
	cout << "Menu: \n1.Add a Passenger\n2.Remove a Passenger\n3.Show List of Passengers\n4.Exit\n";
	
	do {
		choice = menu();
		switch(choice)
		{
			case '1':  cout << "Name: ";
					   cin >> name;
					   cout << "\nFlight: ";
					   cin >> flight;
					   cout << "\nSN: ";
					   cin >> seat_num;
					   cout << "\nPaid: ";
					   cin >> price_paid;
					addPassenger(head, last, name, flight, seat_num, price_paid);
					break;
			case '2': removePassenger(head, last);
					break;
			case '3': showlist(head);
					break;
			default: cout << "\n";
		}
		
		
	} while (choice != '4');
	
	return 0;
}

I know that this is not the best way of use strings and i should use references. But this is simple transformation and it works.

Konstantin2, thank you, will use that!

Pavuluri (4)

very useful content, for more information about the memory allocation and deallocation using free and new refer
http://www.cpptutorials.com/2014/11/memory-allocation-de-allocation-using.html

Hi,

As I said earlier, investigate using smart pointers, and never use new or delete ever again :+)

TheIdeasMan:
Why do you forbid to use "new" and "delete"?
It's not recomended for newbies while they study c++.
But new and delete are part of c++.
Moveover, anybody has rights to use malloc() and free().
You can recommend to not use it. But you have no rights to forbid it.
So you should not say "never use it again".