Stabilize DRM core contracts: fix latent panics, add diagnostics and tests

Production code fixes:
- scheme.rs: replace unwrap() after checked_mul with match binding,
  eliminating a latent panic if code is reordered
- main.rs: log request context_id (PID) on request handling failure
  instead of silently discarding the error
- drivers/amd/display.rs: split silent EDID read fallback into
  separate match arms with log::warn diagnostics for short reads
  and read failures, including byte count and connector index

Test coverage:
- gem.rs: add 4 basic tests for GemManager (create+verify,
  close+verify removal, double-close error, invalid handle error)
This commit is contained in:
2026-04-25 19:52:21 +01:00
parent 4e27bee9bf
commit e62acb2ebb
4 changed files with 72 additions and 13 deletions
@@ -338,7 +338,22 @@ impl DisplayCore {
match self.read_edid_block(connector_index, 0x00) { match self.read_edid_block(connector_index, 0x00) {
Ok(edid) if edid.len() >= 128 => edid, Ok(edid) if edid.len() >= 128 => edid,
Ok(_) | Err(_) => Vec::new(), Ok(short) => {
log::warn!(
"redox-drm: short EDID ({} bytes) from AMD connector {}",
short.len(),
connector_index
);
Vec::new()
}
Err(e) => {
log::warn!(
"redox-drm: EDID read failed for AMD connector {}: {}",
connector_index,
e
);
Vec::new()
}
} }
} }
@@ -121,3 +121,42 @@ impl GemManager {
Ok(self.object(handle)?.gpu_addr) Ok(self.object(handle)?.gpu_addr)
} }
} }
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn create_and_object_exists() {
let mut mgr = GemManager::new();
let h = mgr.create(4096).expect("create should succeed");
let obj = mgr.object(h).expect("object should exist after create");
assert_eq!(obj.handle, h);
assert_eq!(obj.size, 4096);
}
#[test]
fn close_removes_object() {
let mut mgr = GemManager::new();
let h = mgr.create(4096).expect("create should succeed");
mgr.close(h).expect("close should succeed");
assert!(mgr.object(h).is_err(), "object should be gone after close");
}
#[test]
fn double_close_returns_error() {
let mut mgr = GemManager::new();
let h = mgr.create(4096).expect("create should succeed");
mgr.close(h).expect("first close should succeed");
assert!(mgr.close(h).is_err(), "second close should fail");
}
#[test]
fn object_by_invalid_handle_returns_error() {
let mgr = GemManager::new();
assert!(
mgr.object(99999).is_err(),
"querying a non-existent handle should fail"
);
}
}
@@ -129,8 +129,11 @@ fn run() -> Result<()> {
let response = match response { let response = match response {
Ok(response) => response, Ok(response) => response,
Err(_request) => { Err(request) => {
error!("redox-drm: failed to handle request"); error!(
"redox-drm: failed to handle request from context {}",
request.context_id()
);
continue; continue;
} }
}; };
@@ -1011,14 +1011,16 @@ impl DrmScheme {
); );
return Err(Error::new(EINVAL)); return Err(Error::new(EINVAL));
} }
let required_size = (pitch as u64).checked_mul(req.height as u64); let required_size = match (pitch as u64).checked_mul(req.height as u64) {
if required_size.is_none() { Some(s) => s,
warn!( None => {
"redox-drm: ADDFB pitch * height overflows pitch={} height={}", warn!(
pitch, req.height "redox-drm: ADDFB pitch * height overflows pitch={} height={}",
); pitch, req.height
return Err(Error::new(EINVAL)); );
} return Err(Error::new(EINVAL));
}
};
let owned = self let owned = self
.handles .handles
.get(&id) .get(&id)
@@ -1035,10 +1037,10 @@ impl DrmScheme {
warn!("redox-drm: ADDFB handle {} not found: {}", req.handle, e); warn!("redox-drm: ADDFB handle {} not found: {}", req.handle, e);
Error::new(ENOENT) Error::new(ENOENT)
})?; })?;
if required_size.unwrap() > actual_size { if required_size > actual_size {
warn!( warn!(
"redox-drm: ADDFB requires {} bytes but GEM {} is {} bytes", "redox-drm: ADDFB requires {} bytes but GEM {} is {} bytes",
required_size.unwrap(), required_size,
req.handle, req.handle,
actual_size actual_size
); );