Skip to content

list.push_back inside for loop doesn't work, pushes nodes incorrectly #5116

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

Closed
impopular-guy opened this issue May 5, 2025 · 5 comments
Closed

Comments

@impopular-guy
Copy link

Context

In the following code, the commented code works, however push_back inside for loop doesn't work.

package main

import "core:container/intrusive/list"
import "core:fmt"

SimpleNode :: struct {
	node:  list.Node,
	value: int,
}

main :: proc() {
	ll: list.List

	/*
	// THIS WORKS
	sn1 := SimpleNode { value = 1, }
	sn2 := SimpleNode { value = 2, }
	sn3 := SimpleNode { value = 3, }
	sn4 := SimpleNode { value = 4, }
	sn5 := SimpleNode { value = 5, }

	list.push_back(&ll, &sn1.node)
	list.push_back(&ll, &sn2.node)
	list.push_back(&ll, &sn3.node)
	list.push_back(&ll, &sn4.node)
	list.push_back(&ll, &sn5.node)
	*/

	for i in 1 ..= 5 {
		sn := SimpleNode {
			value = i,
		}
		list.push_back(&ll, &sn.node)
	}

	it := list.iterator_head(ll, SimpleNode, "node")
	for n in list.iterate_next(&it) {
		fmt.printf("NODE(%v) ", n.value)
	}
}

Some other points:

  • changing push_back to push_front inside loop, went into an infinite loop

Operating System & Odin Version:

Odin:    dev-2025-04-nightly
OS:      Pop!_OS 22.04 LTS, Linux 6.12.10-76061203-generic
CPU:     AMD Ryzen 5 3550H with Radeon Vega Mobile Gfx  
RAM:     17859 MiB
Backend: LLVM 20.1.1

Expected Behavior

Expected output:

NODE(1) NODE(2) NODE(3) NODE(4) NODE(5)

Current Behavior

Current output

NODE(5)

Failure Information (for bugs)

  • seems like in each loop ll.head is getting updated, i maybe wrong

Steps to Reproduce

Please check code above

Failure Logs

Please include any relevant log snippets or files here.

@Kelimion
Copy link
Member

Kelimion commented May 5, 2025

What you're doing there is take the address of the same SimpleNode literal on the stack every time through the loop each time and trying to push it. If you give it a new node each time, it works:

for i in 1 ..= 5 {
	sn := new_clone(SimpleNode {
		value = i,
	})
	list.push_back(&ll, &sn.node)
}

@impopular-guy
Copy link
Author

impopular-guy commented May 5, 2025

@Kelimion Thanks, i think i understand. However, i am using tracking allocator, and it shows memory leaked. What does that mean, and what to do? (p.s. i am very new to memory management and such)

@Kelimion
Copy link
Member

Kelimion commented May 5, 2025

It means you used memory you didn't free. In your previous example, the memory lived on the stack and so was reused when the procedure exited.

In this case however, we had to allocate memory for each node so that push_back wouldn't keep pointing at the same node over and over again, resulting in the 1 being replaced by the 2, by the 3, and so on.

To free it you can iterate over the list backwards and free it.

// And free
it2 := list.iterator_tail(ll, SimpleNode, "node")
for n in list.iterate_prev(&it2) {
	free(n)
}

Another thing you could do is allocate the nodes from a memory arena and free them all at the same time.

Suffice it to say, I don't believe this to've been a bug.

@impopular-guy
Copy link
Author

thanks, that was lot of learning. You are right, this is not a bug, I should have put it in discussion.

@Kelimion
Copy link
Member

Kelimion commented May 5, 2025

To be fair, you didn't know it wasn't a bug at the time.

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

No branches or pull requests

2 participants