Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 1ce8da3

Browse files
HadrienG2GabrielMajeri
authored andcommitted
Graphics output protocol cleanup (#49)
* Hide reserved byte of BltPixel (else user can trivially change it) * Improve docs and memory safety * Reuse the casts we already have * Apply rustfmt
1 parent c3a2b2f commit 1ce8da3

File tree

1 file changed

+168
-63
lines changed

1 file changed

+168
-63
lines changed

‎src/proto/console/gop.rs

Lines changed: 168 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ pub struct GraphicsOutput {
5454
}
5555

5656
impl GraphicsOutput {
57+
/// Returns information for an available graphics mode that the graphics
58+
/// device and the set of active video output devices supports.
5759
fn query_mode(&self, index: u32) -> Result<Mode> {
5860
let mut info_sz = 0;
5961
let mut info = ptr::null();
@@ -68,7 +70,7 @@ impl GraphicsOutput {
6870
})
6971
}
7072

71-
/// Returns information about available graphics modes and output devices.
73+
/// Returns information about all available graphics modes.
7274
pub fn modes<'a>(&'a self) -> impl Iterator<Item = Mode> + 'a {
7375
ModeIter {
7476
gop: self,
@@ -77,7 +79,8 @@ impl GraphicsOutput {
7779
}
7880
}
7981

80-
/// Sets the current graphics mode.
82+
/// Sets the video device into the specified mode, clearing visible portions
83+
/// of the output display to black.
8184
///
8285
/// This function **will** invalidate the current framebuffer and change the current mode.
8386
pub fn set_mode(&mut self, mode: &Mode) -> Result<()> {
@@ -94,61 +97,144 @@ impl GraphicsOutput {
9497
color,
9598
dest: (dest_x, dest_y),
9699
dims: (width, height),
97-
} => (self.blt)(
98-
self,
99-
&color as *const _ as usize,
100-
0,
101-
0,
102-
0,
103-
dest_x,
104-
dest_y,
105-
width,
106-
height,
107-
1,
108-
).into(),
100+
} => {
101+
self.check_framebuffer_region((dest_x, dest_y), (width, height));
102+
(self.blt)(
103+
self,
104+
&color as *const _ as usize,
105+
0,
106+
0,
107+
0,
108+
dest_x,
109+
dest_y,
110+
width,
111+
height,
112+
0,
113+
).into()
114+
}
109115
BltOp::VideoToBltBuffer {
110116
buffer,
111-
stride,
112117
src: (src_x, src_y),
113-
dest: (dest_x, dest_y),
118+
dest: dest_region,
114119
dims: (width, height),
115-
} => (self.blt)(
116-
self,
117-
buffer.as_mut_ptr() as usize,
118-
1,
119-
src_x,
120-
src_y,
121-
dest_x,
122-
dest_y,
123-
width,
124-
height,
125-
stride,
126-
).into(),
120+
} => {
121+
self.check_framebuffer_region((src_x, src_y), (width, height));
122+
self.check_blt_buffer_region(dest_region, (width, height), buffer.len());
123+
match dest_region {
124+
BltRegion::Full => (self.blt)(
125+
self,
126+
buffer.as_mut_ptr() as usize,
127+
1,
128+
src_x,
129+
src_y,
130+
0,
131+
0,
132+
width,
133+
height,
134+
0,
135+
).into(),
136+
BltRegion::SubRectangle {
137+
coords: (dest_x, dest_y),
138+
px_stride,
139+
} => (self.blt)(
140+
self,
141+
buffer.as_mut_ptr() as usize,
142+
1,
143+
src_x,
144+
src_y,
145+
dest_x,
146+
dest_y,
147+
width,
148+
height,
149+
px_stride * core::mem::size_of::<BltPixel>(),
150+
).into(),
151+
}
152+
}
127153
BltOp::BufferToVideo {
128154
buffer,
129-
stride,
130-
src: (src_x, src_y),
155+
src: src_region,
131156
dest: (dest_x, dest_y),
132157
dims: (width, height),
133-
} => (self.blt)(
134-
self,
135-
buffer.as_ptr() as usize,
136-
2,
137-
src_x,
138-
src_y,
139-
dest_x,
140-
dest_y,
141-
width,
142-
height,
143-
stride,
144-
).into(),
158+
} => {
159+
self.check_blt_buffer_region(src_region, (width, height), buffer.len());
160+
self.check_framebuffer_region((dest_x, dest_y), (width, height));
161+
match src_region {
162+
BltRegion::Full => (self.blt)(
163+
self,
164+
buffer.as_ptr() as usize,
165+
2,
166+
0,
167+
0,
168+
dest_x,
169+
dest_y,
170+
width,
171+
height,
172+
0,
173+
).into(),
174+
BltRegion::SubRectangle {
175+
coords: (src_x, src_y),
176+
px_stride,
177+
} => (self.blt)(
178+
self,
179+
buffer.as_ptr() as usize,
180+
2,
181+
src_x,
182+
src_y,
183+
dest_x,
184+
dest_y,
185+
width,
186+
height,
187+
px_stride * core::mem::size_of::<BltPixel>(),
188+
).into(),
189+
}
190+
}
145191
BltOp::VideoToVideo {
146192
src: (src_x, src_y),
147193
dest: (dest_x, dest_y),
148194
dims: (width, height),
149-
} => (self.blt)(
150-
self, 0usize, 3, src_x, src_y, dest_x, dest_y, width, height, 0,
151-
).into(),
195+
} => {
196+
self.check_framebuffer_region((src_x, src_y), (width, height));
197+
self.check_framebuffer_region((dest_x, dest_y), (width, height));
198+
(self.blt)(
199+
self, 0usize, 3, src_x, src_y, dest_x, dest_y, width, height, 0,
200+
).into()
201+
}
202+
}
203+
}
204+
205+
/// Memory-safety check for accessing a region of the framebuffer
206+
fn check_framebuffer_region(&self, coords: (usize, usize), dims: (usize, usize)) {
207+
let (width, height) = self.current_mode_info().resolution();
208+
assert!(
209+
coords.0.saturating_add(dims.0) <= width,
210+
"Horizontal framebuffer coordinate out of bounds"
211+
);
212+
assert!(
213+
coords.1.saturating_add(dims.1) <= height,
214+
"Vertical framebuffer coordinate out of bounds"
215+
);
216+
}
217+
218+
/// Memory-safety check for accessing a region of a user-provided buffer
219+
fn check_blt_buffer_region(&self, region: BltRegion, dims: (usize, usize), buf_length: usize) {
220+
match region {
221+
BltRegion::Full => assert!(
222+
dims.0.saturating_add(dims.1.saturating_mul(dims.0)) <= buf_length,
223+
"BltBuffer access out of bounds"
224+
),
225+
BltRegion::SubRectangle {
226+
coords: (x, y),
227+
px_stride,
228+
} => {
229+
assert!(
230+
x.saturating_add(dims.0) <= px_stride,
231+
"Horizontal BltBuffer coordinate out of bounds"
232+
);
233+
assert!(
234+
y.saturating_add(dims.1).saturating_mul(px_stride) <= buf_length,
235+
"Vertical BltBuffer coordinate out of bounds"
236+
);
237+
}
152238
}
153239
}
154240

@@ -165,6 +251,10 @@ impl GraphicsOutput {
165251
/// It is also the callers responsibilty to use volatile memory accesses,
166252
/// otherwise they could be optimized to nothing.
167253
pub unsafe fn frame_buffer(&mut self) -> &mut [u8] {
254+
assert!(
255+
self.mode.info.format != PixelFormat::BltOnly,
256+
"Cannot access the framebuffer in a Blt-only mode"
257+
);
168258
let data = self.mode.fb_address as *mut u8;
169259
let len = self.mode.fb_size;
170260

@@ -321,7 +411,7 @@ pub struct BltPixel {
321411
pub blue: u8,
322412
pub green: u8,
323413
pub red: u8,
324-
pubreserved: u8,
414+
_reserved: u8,
325415
}
326416

327417
impl BltPixel {
@@ -331,7 +421,7 @@ impl BltPixel {
331421
red,
332422
green,
333423
blue,
334-
reserved: 0,
424+
_reserved: 0,
335425
}
336426
}
337427
}
@@ -342,11 +432,30 @@ impl From<u32> for BltPixel {
342432
blue: (color & 0x00_00_FF) as u8,
343433
green: (color & 0x00_FF_00 >> 8) as u8,
344434
red: (color & 0xFF_00_00 >> 16) as u8,
345-
reserved: 0,
435+
_reserved: 0,
346436
}
347437
}
348438
}
349439

440+
/// Region of the BltBuffer which we are operating on
441+
///
442+
/// Some Blt operations can operate on either the full BltBuffer or a
443+
/// sub-rectangle of it, but require the stride to be known in the latter case.
444+
#[derive(Clone, Copy, Debug)]
445+
pub enum BltRegion {
446+
/// Operate on the full BltBuffer
447+
Full,
448+
449+
/// Operate on a sub-rectangle of the BltBuffer
450+
SubRectangle {
451+
/// Coordinate of the rectangle in the BltBuffer
452+
coords: (usize, usize),
453+
454+
/// Stride (length of each row of the BltBuffer) in **pixels**
455+
px_stride: usize,
456+
},
457+
}
458+
350459
/// Blit operation to perform.
351460
#[derive(Debug)]
352461
pub enum BltOp<'a> {
@@ -363,37 +472,33 @@ pub enum BltOp<'a> {
363472
VideoToBltBuffer {
364473
/// Buffer into which to copy data.
365474
buffer: &'a mut [BltPixel],
366-
/// The stride is the width / number of bytes in each row of the user-provided buffer.
367-
stride: usize,
368-
/// The coordinate of the source rectangle, in the frame buffer.
475+
/// Coordinates of the source rectangle, in the frame buffer.
369476
src: (usize, usize),
370-
/// The coordinate of the destination rectangle, in the user-provided buffer.
371-
dest: (usize,usize),
372-
/// The width / height of the rectangles.
477+
/// Location of the destination rectangle in the user-provided buffer
478+
dest: BltRegion,
479+
/// Width / height of the rectangles.
373480
dims: (usize, usize),
374481
},
375482
/// Write data from the buffer to the video rectangle.
376483
/// Delta must be the stride (count of bytes in a row) of the buffer.
377484
BufferToVideo {
378485
/// Buffer from which to copy data.
379486
buffer: &'a [BltPixel],
380-
/// The stride is the width / number of bytes in each row of the user-provided buffer.
381-
stride: usize,
382-
/// The coordinate of the source rectangle, in the user-provided buffer.
383-
src: (usize, usize),
384-
/// The coordinate of the destination rectangle, in the frame buffer.
487+
/// Location of the source rectangle in the user-provided buffer.
488+
src: BltRegion,
489+
/// Coordinates of the destination rectangle, in the frame buffer.
385490
dest: (usize, usize),
386-
/// The width / height of the rectangles.
491+
/// Width / height of the rectangles.
387492
dims: (usize, usize),
388493
},
389494
/// Copy from the source rectangle in video memory to
390495
/// the destination rectangle, also in video memory.
391496
VideoToVideo {
392-
/// The coordinate of the source rectangle, in the frame buffer.
497+
/// Coordinates of the source rectangle, in the frame buffer.
393498
src: (usize, usize),
394-
/// The coordinate of the destination rectangle, also in the frame buffer.
499+
/// Coordinates of the destination rectangle, also in the frame buffer.
395500
dest: (usize, usize),
396-
/// The width / height of the rectangles.
501+
/// Width / height of the rectangles.
397502
dims: (usize, usize),
398503
},
399504
}

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /