Don't derive Copy/Clone for Frame so we can be sure that it's free #52

phil-opp · Nov 19, 2015

If a Frame would be clonable or even Copy, the frame could be freed (e.g. passed to deallocate_frame) and used thereafter. Or it could be freed multiple times.

phil-opp merged commit cefe95d into master Nov 19, 2015
2 checks passed

2 checks passed

Details continuous-integration/travis-ci/pr The Travis CI build passed

Details continuous-integration/travis-ci/push The Travis CI build passed

phil-opp deleted the remove_copy_for_frame branch Nov 19, 2015

      First we create a memory module with a `Frame` type (`src/memory/mod.rs`):
      ```rust
-    -#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
+    +#[derive(Debug, PartialEq, Eq, PartialOrd, Ord)]
      pub struct Frame {
          number: usize,
      }
      ```
-    -(Don't forget to add the `mod memory` line to `src/lib.rs`.) Instead of e.g. the start address, we just store the frame number. We use `usize` here since the number of frames depends on the memory size. The long `derive` line makes frames printable, clonable, and comparable.
+    +(Don't forget to add the `mod memory` line to `src/lib.rs`.) Instead of e.g. the start address, we just store the frame number. We use `usize` here since the number of frames depends on the memory size. The long `derive` line makes frames printable and comparable.
+    +
+    +_Update_: In a previous version, the `Clone` and `Copy` traits were derived, too. [This was removed][PR 52] to make the allocator interface safer.
+    +[PR 52]: https://github.com/phil-opp/blog_os/pull/52
      To make it easy to get the corresponding frame for a physical address, we add a `containing_address` method:
      ```rust
      fn allocate_frame(&mut self) -> Option<Frame> {
          if let Some(area) = self.current_area {
-    -        let frame = self.next_free_frame;
+    +        // "clone" the frame to return it if it's free. Frame doesn't
+    +        // implement Clone, but we can construct an identical frame.
+    +        let frame = Frame{ number: self.next_free_frame.number };
              // the last frame of the current area
              let current_area_last_frame = {

@@ @@ -51,7 +51,9 @@ impl AreaFrameAllocator { @@
      impl FrameAllocator for AreaFrameAllocator {
          fn allocate_frame(&mut self) -> Option<Frame> {
              if let Some(area) = self.current_area {
-    -            let frame = self.next_free_frame;
+    +            // "clone" the frame to return it if it's free. Frame doesn't
+    +            // implement Clone, but we can construct an identical frame.
+    +            let frame = Frame{ number: self.next_free_frame.number };
                  // the last frame of the current area
                  let current_area_last_frame = {

@@ @@ -4,7 +4,7 @@ mod area_frame_allocator; @@
      pub const PAGE_SIZE: usize = 4096;
-    -#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
+    +#[derive(Debug, PartialEq, Eq, PartialOrd, Ord)]
      pub struct Frame {
          number: usize,
      }

phil-opp/blog_os

Don't derive Copy/Clone for Frame so we can be sure that it's free #52

Labels

Milestone

Assignee

1 participant

2 checks passed

phil-opp/blog_os

Don't derive Copy/Clone for Frame so we can be sure that it's free #52

Labels

Milestone

Assignee

1 participant

All checks have passed

2 checks passed

All checks have passed