Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Comments for "https://os.phil-opp.com/heap-allocation/" #627

Closed
utterances-bot opened this issue Jun 26, 2019 · 25 comments
Closed

Comments for "https://os.phil-opp.com/heap-allocation/" #627

utterances-bot opened this issue Jun 26, 2019 · 25 comments

Comments

@utterances-bot
Copy link

utterances-bot commented Jun 26, 2019

This is a general purpose comment thread for the Heap Allocation post.

Copy link

Thank you I was waiting this part !

@phil-opp phil-opp changed the title https://os.phil-opp.com/heap-allocation/ Comments for "https://os.phil-opp.com/heap-allocation/" Jun 28, 2019
Copy link

shakram02 commented Nov 16, 2019

Hey Philipp, thank you for the great post ( as always ✌️ )

I wanted to note that I think now it's possible to move the allocator to its own module and out of lib.rs because of this PR.

Can't wait for your next tutorial!

@toothbrush7777777
Copy link
Contributor

@phil-opp As @shakram02 noted, rust-lang/rust#62735 now makes it possible to define the global allocator in a submodule.

@phil-opp
Copy link
Owner

phil-opp commented Jan 8, 2020

@shakram02 @toothbrush7777777 Thank you for reporting this and sorry for the late reply. I created #714 to move the #[global_allocator] into the allocator module in the post-10 branch and I will create a second pull request to update the blog text in a moment.

Edit: Blog update in #715.

@Menschenkindlein
Copy link
Contributor

The comment in the first code block in https://os.phil-opp.com/heap-allocation/#the-global-allocator-attribute should be // in src/lib.rs.

@phil-opp
Copy link
Owner

@Menschenkindlein We recently moved the definition to the allocator module in #714. Try updating your Rust nightly if you get an error.

@Menschenkindlein
Copy link
Contributor

Menschenkindlein commented Jan 28, 2020

I guess it doesn't need allocator:: then, does it?

@phil-opp
Copy link
Owner

Ah, good catch! Fixed in #728.

Copy link

GuillaumeDIDIER commented Feb 20, 2020

How do you handle memory allocation failure gracefully in here ? (If my users starts forkbombing I’d like to avoid crashing the kernel and instead tell the user nope, your fork is a spoon)

@phil-opp
Copy link
Owner

Currently the only way is to use the try_reserve method before appending to collections. I linked the method for Vec, but it also exists for the other collection types. These methods were introduced with the fallible collection allocation 1.0 RFC.

Alternatively, you could create your own alloc library that returns a result instead of panicking for all collection methods. Or, try to get unwinding working with your kernel so that you can catch the oom panic.

Copy link

Going down the rabbit hole in stumbled onto https:/vcombey/fallible_collections not sure if other such crate exist, but this looks like a reasonable thing to use, until hopefully the original alloc crate decides on how to play nice with people who need to handle memory allocation errors. (The ability to have the allocator specify an Error type with the default being ! would imho be very nice)

@phil-opp
Copy link
Owner

Going down the rabbit hole in stumbled onto https:/vcombey/fallible_collections

Looks useful!

[…] until hopefully the original alloc crate decides on how to play nice with people who need to handle memory allocation errors. (The ability to have the allocator specify an Error type with the default being ! would imho be very nice)

I'm not aware of any proposals to change the global allocator, but there is the allocators working group that works on making collections parametrizable over the separate Alloc trait. The idea is that you can write Vec<T, MyAllocatorType> to make the Vec instance use a different allocator than the global allocator. There was also a discussion on an associated error type: rust-lang/wg-allocators#23.

Copy link

Hello. First, thank you for this amazing tutorial. I really enjoy it.

I do have a build error when using the linked-list-allocator:

error[E0432]: unresolved import `alloc::alloc::AllocRef`

It seems that alloc does not have AllocRef module any more.

@phil-opp
Copy link
Owner

@n-osborne Thanks! The AllocRef trait was renamed from Alloc recently. Try updating your nightly Rust version to resolve the error.

Copy link

@phil-opp Thanks, it works like a charm.

Copy link

Hi phil-oop, I'm really enjoy your posts, and I have a question: Why alloc method in GlobalAlloc trait returns a mut pointer which type is u8, not usize? I think that data type u8 can not represent a full memory address in the machine.

@bjorn3
Copy link
Contributor

bjorn3 commented May 12, 2020

@imaginezz *mut T contains a full memory address for any type T. The choice for u8 as T to form *mut u8 is somewhat arbitrary. It could just as well have been *mut u16 or *mut ().

Copy link
Contributor

toothbrush7777777 commented May 12, 2020

@imaginezz A usize is "The pointer-sized unsigned integer type." *mut u8 is a raw pointer to a u8, not a u8-sized pointer.

@imaginezz
Copy link

imaginezz commented May 12, 2020

Yes, it is! *u8 is the pointer to u8 data type like C. Thank you @toothbrush7777777 @bjorn3 very much!

@mitnk
Copy link

mitnk commented May 22, 2020

The tests in tests/heap_allocation.rs can be improved a bit, since all 3 cases are passing for a following broken BumpAllocator implementation:

unsafe impl GlobalAlloc for Locked<BumpAllocator> {
    unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
        self.lock().heap_start as *mut u8
    }

    unsafe fn dealloc(&self, _ptr: *mut u8, _layout: Layout) {
    }
}

Change simple_allocation case like this would catch this issue:

    let heap_value1 = Box::new(41);
    let heap_value2 = Box::new(13);
    assert_eq!(*heap_value1, 41);
    assert_eq!(*heap_value2, 13);

phil-opp added a commit that referenced this issue May 22, 2020
@phil-opp
Copy link
Owner

@mitnk Good idea! I updated the test as you suggested in b567d06 and 59a7f96.

Copy link

dalnefre commented Jun 4, 2020

In your previous post you say:

the create_example_mapping function is only a temporary testing function and will be removed after this post, so it is ok. To remind us of the unsafety, we put a FIXME comment on the line.

However, the code still appears in this and future repos. Was this intentional?

@phil-opp
Copy link
Owner

phil-opp commented Oct 8, 2020

@dalnefre Sorry, your comment somehow slipped through my notifications. You're completely right, the method should be removed from the code of the subsequent posts. I fixed this in 3a230d0. Thanks for reporting!

Copy link

Thanks for the great tutorials! I am wanting to expand, and was thinking about creating a basic shell (like you announced in edition-3), however kinda confused on how to implement something like that, like how to get a constant stream of key-strokes. Would you have any suggestions?

Or maybe ideas on how to expand on these posts? Challenges? Thanks!

@toothbrush7777777
Copy link
Contributor

toothbrush7777777 commented Feb 16, 2021

Thanks for the great tutorials! I am wanting to expand, and was thinking about creating a basic shell (like you announced in edition-3), however kinda confused on how to implement something like that, like how to get a constant stream of key-strokes. Would you have any suggestions?

You should read https://os.phil-opp.com/async-await/ for an example.
A crate you can use is pc-keyboard.

Repository owner locked and limited conversation to collaborators Jun 12, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests