Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed bug in linked list where the lastnode wasn't being updated when… #383

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

stevendargaville
Copy link
Contributor

… the last value was popped off the end of the list. Causes a segfault the next time you try and access the lastnode after a pop_last.

… the last value was popped off the end of the list
@stephankramer
Copy link
Contributor

I'm a little confused: if you don't update lastnode then it presumably still points to node (the last current node it has found by looping through the list), which has now been deallocated?

The issue I see with the current code is that if the length of the current list is one, it never goes into the loop, so prev_node is never initialised. There should probably be a special case where it leaves a valid 0-length list behind in that case.

@stevendargaville
Copy link
Contributor Author

I'm a little confused: if you don't update lastnode then it presumably still points to node (the last current node it has found by looping through the list), which has now been deallocated?

Yes, so I think that if you call pop_last and then immediately call insert for a non-zero length list, line 264 (node => list%lastnode) in insert will segfault?

The issue I see with the current code is that if the length of the current list is one, it never goes into the loop, so prev_node is never initialised. There should probably be a special case where it leaves a valid 0-length list behind in that case.

Yes you're right, there should be a guard on the assignment of lastnode with a list of size one, I'll add that now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants