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

🗑️Fix the stack overflow caused by a static array of String #46

Merged
merged 1 commit into from
May 26, 2024

Conversation

joajfreitas
Copy link
Collaborator

No description provided.

@joajfreitas joajfreitas changed the title 🗑️Fix the stack overflow cause by a static array of String 🗑️Fix the stack overflow caused by a static array of String May 26, 2024
Comment on lines +226 to +229
mem: vec![0; 65536],
cartridge: Vec::new(),
bootrom: include_bytes!("../dmg0.bin"),
code_listing: [ARRAY_REPEAT_VALUE; 0xffff + 1],
code_listing: vec![ARRAY_REPEAT_VALUE; 0xffff + 1],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

didn't know the vec! macro also supported the [x; N] syntax. Nice.

@@ -242,7 +242,7 @@ impl Memory {
&mut self.mem[range]
}

pub fn code_listing(&self) -> &[Option<String>; 0xffff + 1] {
pub fn code_listing(&self) -> &Vec<Option<String>> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw the return type can also be &[Option<String>] because Vec<T> implements the Deref and DerefMut traits with type Target = T (which is String in our case)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@joajfreitas joajfreitas merged commit bfb58ee into main May 26, 2024
1 check passed
@joajfreitas joajfreitas deleted the fix_stack_overflow branch May 26, 2024 18:58
@joajfreitas joajfreitas self-assigned this Aug 22, 2024
@joajfreitas joajfreitas added the bug Something isn't working label Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants