Returning FrameAlreadyMapped error when it shouldn't about x86_64 HOT 10 CLOSED

I think there are some users of the mapping function that expect it to return an error if the entry is already present even if that entry contains the same frame. Changing this behavior would be a breaking change and a subtle one.

from x86_64.

We get this error when we try to change the page flags, which isn't right, we should only return this error if the frame address's are different.

Is there a reason why you aren't using Mapper::update_flags if you only want to change the flags and not the frame?

Also why do we return the user inputted frame and not the previously mapped frame ?

See #117 and #118.

from x86_64.

Sorry, I didn't see this function !
But maybe we should use the update_flags implicitly ?

from x86_64.

Yes I'm okay, but also why do we have Mapper::update_flags but not Mapper::get_flags, I think it would be a cool addition

from x86_64.

Yes I'm okay, but also why do we have Mapper::update_flags but not Mapper::get_flags, I think it would be a cool addition

That sounds reasonable to me. Feel free to open a PR :)

from x86_64.

Yes I'm okay, but also why do we have Mapper::update_flags but not Mapper::get_flags, I think it would be a cool addition

The Mapper trait is about updating the page tables, i.e. creating or changing some mappings. We have a separate trait called Translate for inspecting the page tables without modification. For example, you can use the Translate::translate to get all the information about a virtual page, including its flags.

from x86_64.

Okay thanks !
Well I guess my issue is useless... So I can close it...
Although I think it's not really straight forward...

from x86_64.

Although I think it's not really straight forward...

Do you think it's mainly a documentation issue or do you see a way how we could make the API itself simpler?

from x86_64.

The API should change, because I think the update_flags and get_flags should be on the same object
But we can also just add in comments for the update_flags function:
/// See Translate::get_flags to get the flags on a specific page/frame

from x86_64.

You can check the docs for a specific page table type to see all supported methods, e.g. https://docs.rs/x86_64/latest/x86_64/structures/paging/mapper/struct.OffsetPageTable.html. Keeping the traits separate allows creating custom read-only or write-only page table types.

But we can also just add in comments for the update_flags function:
/// See Translate::get_flags to get the flags on a specific page/frame

I agree that such a comment would be useful. Could you submit a PR?

from x86_64.