From a8600a00b601f2cbdebd67a43b9f6b5789aaec35 Mon Sep 17 00:00:00 2001
From: Philipp Oppermann <dev@phil-opp.com>
Date: Thu, 19 Nov 2015 19:44:06 +0100
Subject: [PATCH] Don't derive Copy/Clone for Frame so we can be sure that it's
 free

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.
---
 posts/2015-11-15-allocating-frames.md | 11 ++++++++---
 src/memory/area_frame_allocator.rs    |  4 +++-
 src/memory/mod.rs                     |  2 +-
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/posts/2015-11-15-allocating-frames.md b/posts/2015-11-15-allocating-frames.md
index 4f87db4a..49ffc2f1 100644
--- a/posts/2015-11-15-allocating-frames.md
+++ b/posts/2015-11-15-allocating-frames.md
@@ -203,12 +203,15 @@ So let's start implementing our memory map based frame allocator.
 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:
 
@@ -256,7 +259,9 @@ To implement the `FrameAllocator` trait, we need to implement the `allocate_fram
 ```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 = {
diff --git a/src/memory/area_frame_allocator.rs b/src/memory/area_frame_allocator.rs
index edc67651..86563e63 100644
--- a/src/memory/area_frame_allocator.rs
+++ b/src/memory/area_frame_allocator.rs
@@ -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 = {
diff --git a/src/memory/mod.rs b/src/memory/mod.rs
index 385c574f..7bfc85d8 100644
--- a/src/memory/mod.rs
+++ b/src/memory/mod.rs
@@ -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,
 }