Skip to content

Commit add55d9

Browse files
jrvanwhyjrvanwhy
committed
Add safe constructors for CommandReturn to libtock_unittest.
`libtock_platform` does not provide safe constructors for `CommandReturn`, as in a process binary it is supposed to only be created by invocations of the Command system call. However, fake drivers for unit tests (as well as many unit tests) have a need to create their own `CommandReturn` values. To avoid a proliferation of `unsafe`, I added safe constructors for `CommandReturn` to `libtock_unittest`. For testing purposes, I added unconditional `Debug` derives to some of the `libtock_platform` types. These are meant only for use in host-based tests: embedded code should use a lighter-weight solution (we're currently trying out `ufmt` for this).
1 parent 38915e1 commit add55d9

File tree

6 files changed

+229
-43
lines changed

6 files changed

+229
-43
lines changed

‎platform/src/command_return.rs

+16-27
Original file line numberDiff line numberDiff line change
@@ -4,33 +4,21 @@ use core::mem::transmute;
44

55
/// The response type from `command`. Can represent a successful value or a
66
/// failure.
7-
#[derive(Clone, Copy)]
7+
#[derive(Clone, Copy, Debug)]
88
pub struct CommandReturn {
99
return_variant: ReturnVariant,
10-
// r1, r2, and r3 should only contain 32-bit values. However, these are
11-
// converted directly from usizes returned by RawSyscalls::four_arg_syscall.
12-
// To avoid casting twice (both when converting to a Command Return and when
13-
// calling a get_*() function), we store the usizes directly. Then using the
14-
// CommandReturn only involves one conversion for each of r1, r2, and r3,
15-
// performed in the get_*() functions.
1610

1711
// Safety invariant on r1: If return_variant is failure variant, r1 must be
1812
// a valid ErrorCode.
19-
r1: usize,
20-
r2: usize,
21-
r3: usize,
13+
r1: u32,
14+
r2: u32,
15+
r3: u32,
2216
}
2317

2418
impl CommandReturn {
2519
/// # Safety
2620
/// If return_variant is a failure variant, r1 must be a valid ErrorCode.
27-
#[cfg(test)] // Will be removed when command() is implemented.
28-
pub(crate) unsafe fn new(
29-
return_variant: ReturnVariant,
30-
r1: usize,
31-
r2: usize,
32-
r3: usize,
33-
) -> Self {
21+
pub unsafe fn new(return_variant: ReturnVariant, r1: u32, r2: u32, r3: u32) -> Self {
3422
CommandReturn {
3523
return_variant,
3624
r1,
@@ -115,7 +103,7 @@ impl CommandReturn {
115103
if !self.is_failure_u32() {
116104
return None;
117105
}
118-
Some((unsafe { transmute(self.r1 as u16) }, self.r2 as u32))
106+
Some((unsafe { transmute(self.r1 as u16) }, self.r2))
119107
}
120108

121109
/// Returns the error code and return values if this CommandReturn is of
@@ -124,11 +112,7 @@ impl CommandReturn {
124112
if !self.is_failure_2_u32() {
125113
return None;
126114
}
127-
Some((
128-
unsafe { transmute(self.r1 as u16) },
129-
self.r2 as u32,
130-
self.r3 as u32,
131-
))
115+
Some((unsafe { transmute(self.r1 as u16) }, self.r2, self.r3))
132116
}
133117

134118
/// Returns the error code and return value if this CommandReturn is of type
@@ -148,15 +132,15 @@ impl CommandReturn {
148132
if !self.is_success_u32() {
149133
return None;
150134
}
151-
Some(self.r1 as u32)
135+
Some(self.r1)
152136
}
153137

154138
/// Returns the values if this CommandReturn is of type Success with 2 u32.
155139
pub fn get_success_2_u32(&self) -> Option<(u32, u32)> {
156140
if !self.is_success_2_u32() {
157141
return None;
158142
}
159-
Some((self.r1 as u32, self.r2 as u32))
143+
Some((self.r1, self.r2))
160144
}
161145

162146
/// Returns the value if this CommandReturn is of type Success with u64.
@@ -172,7 +156,7 @@ impl CommandReturn {
172156
if !self.is_success_3_u32() {
173157
return None;
174158
}
175-
Some((self.r1 as u32, self.r2 as u32, self.r3 as u32))
159+
Some((self.r1, self.r2, self.r3))
176160
}
177161

178162
/// Returns the values if this CommandReturn is of type Success with u32 and
@@ -181,7 +165,12 @@ impl CommandReturn {
181165
if !self.is_success_u32_u64() {
182166
return None;
183167
}
184-
Some((self.r1 as u32, self.r2 as u64 + ((self.r3 as u64) << 32)))
168+
Some((self.r1, self.r2 as u64 + ((self.r3 as u64) << 32)))
169+
}
170+
171+
/// Returns the register values used to create this command.
172+
pub fn raw_values(&self) -> (ReturnVariant, u32, u32, u32) {
173+
(self.return_variant, self.r1, self.r2, self.r3)
185174
}
186175

187176
/// Returns the return variant of this command.

‎platform/src/command_return_tests.rs

+44-4
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ fn failure() {
55
let command_return = unsafe {
66
CommandReturn::new(
77
return_variant::FAILURE,
8-
ErrorCode::Reserve as usize,
8+
ErrorCode::Reserve as u32,
99
1002,
1010
1003,
1111
)
@@ -29,6 +29,10 @@ fn failure() {
2929
assert_eq!(command_return.get_success_u64(), None);
3030
assert_eq!(command_return.get_success_3_u32(), None);
3131
assert_eq!(command_return.get_success_u32_u64(), None);
32+
assert_eq!(
33+
command_return.raw_values(),
34+
(return_variant::FAILURE, 5, 1002, 1003)
35+
);
3236
assert_eq!(command_return.return_variant(), return_variant::FAILURE);
3337
}
3438

@@ -37,7 +41,7 @@ fn failure_u32() {
3741
let command_return = unsafe {
3842
CommandReturn::new(
3943
return_variant::FAILURE_U32,
40-
ErrorCode::Off as usize,
44+
ErrorCode::Off as u32,
4145
1002,
4246
1003,
4347
)
@@ -64,6 +68,10 @@ fn failure_u32() {
6468
assert_eq!(command_return.get_success_u64(), None);
6569
assert_eq!(command_return.get_success_3_u32(), None);
6670
assert_eq!(command_return.get_success_u32_u64(), None);
71+
assert_eq!(
72+
command_return.raw_values(),
73+
(return_variant::FAILURE_U32, 4, 1002, 1003)
74+
);
6775
assert_eq!(command_return.return_variant(), return_variant::FAILURE_U32);
6876
}
6977

@@ -72,7 +80,7 @@ fn failure_2_u32() {
7280
let command_return = unsafe {
7381
CommandReturn::new(
7482
return_variant::FAILURE_2_U32,
75-
ErrorCode::Already as usize,
83+
ErrorCode::Already as u32,
7684
1002,
7785
1003,
7886
)
@@ -99,6 +107,10 @@ fn failure_2_u32() {
99107
assert_eq!(command_return.get_success_u64(), None);
100108
assert_eq!(command_return.get_success_3_u32(), None);
101109
assert_eq!(command_return.get_success_u32_u64(), None);
110+
assert_eq!(
111+
command_return.raw_values(),
112+
(return_variant::FAILURE_2_U32, 3, 1002, 1003)
113+
);
102114
assert_eq!(
103115
command_return.return_variant(),
104116
return_variant::FAILURE_2_U32
@@ -110,7 +122,7 @@ fn failure_u64() {
110122
let command_return = unsafe {
111123
CommandReturn::new(
112124
return_variant::FAILURE_U64,
113-
ErrorCode::Busy as usize,
125+
ErrorCode::Busy as u32,
114126
0x1002,
115127
0x1003,
116128
)
@@ -137,6 +149,10 @@ fn failure_u64() {
137149
assert_eq!(command_return.get_success_u64(), None);
138150
assert_eq!(command_return.get_success_3_u32(), None);
139151
assert_eq!(command_return.get_success_u32_u64(), None);
152+
assert_eq!(
153+
command_return.raw_values(),
154+
(return_variant::FAILURE_U64, 2, 0x1002, 0x1003)
155+
);
140156
assert_eq!(command_return.return_variant(), return_variant::FAILURE_U64);
141157
}
142158

@@ -162,6 +178,10 @@ fn success() {
162178
assert_eq!(command_return.get_success_u64(), None);
163179
assert_eq!(command_return.get_success_3_u32(), None);
164180
assert_eq!(command_return.get_success_u32_u64(), None);
181+
assert_eq!(
182+
command_return.raw_values(),
183+
(return_variant::SUCCESS, 1001, 1002, 1003)
184+
);
165185
assert_eq!(command_return.return_variant(), return_variant::SUCCESS);
166186
}
167187

@@ -188,6 +208,10 @@ fn success_u32() {
188208
assert_eq!(command_return.get_success_u64(), None);
189209
assert_eq!(command_return.get_success_3_u32(), None);
190210
assert_eq!(command_return.get_success_u32_u64(), None);
211+
assert_eq!(
212+
command_return.raw_values(),
213+
(return_variant::SUCCESS_U32, 1001, 1002, 1003)
214+
);
191215
assert_eq!(command_return.return_variant(), return_variant::SUCCESS_U32);
192216
}
193217

@@ -214,6 +238,10 @@ fn success_2_u32() {
214238
assert_eq!(command_return.get_success_u64(), None);
215239
assert_eq!(command_return.get_success_3_u32(), None);
216240
assert_eq!(command_return.get_success_u32_u64(), None);
241+
assert_eq!(
242+
command_return.raw_values(),
243+
(return_variant::SUCCESS_2_U32, 1001, 1002, 1003)
244+
);
217245
assert_eq!(
218246
command_return.return_variant(),
219247
return_variant::SUCCESS_2_U32
@@ -246,6 +274,10 @@ fn success_u64() {
246274
);
247275
assert_eq!(command_return.get_success_3_u32(), None);
248276
assert_eq!(command_return.get_success_u32_u64(), None);
277+
assert_eq!(
278+
command_return.raw_values(),
279+
(return_variant::SUCCESS_U64, 0x1001, 0x1002, 1003)
280+
);
249281
assert_eq!(command_return.return_variant(), return_variant::SUCCESS_U64);
250282
}
251283

@@ -272,6 +304,10 @@ fn success_3_u32() {
272304
assert_eq!(command_return.get_success_u64(), None);
273305
assert_eq!(command_return.get_success_3_u32(), Some((1001, 1002, 1003)));
274306
assert_eq!(command_return.get_success_u32_u64(), None);
307+
assert_eq!(
308+
command_return.raw_values(),
309+
(return_variant::SUCCESS_3_U32, 1001, 1002, 1003)
310+
);
275311
assert_eq!(
276312
command_return.return_variant(),
277313
return_variant::SUCCESS_3_U32
@@ -304,6 +340,10 @@ fn success_u32_u64() {
304340
command_return.get_success_u32_u64(),
305341
Some((1001, 0x0000_1003_0000_1002))
306342
);
343+
assert_eq!(
344+
command_return.raw_values(),
345+
(return_variant::SUCCESS_U32_U64, 1001, 0x1002, 0x1003)
346+
);
307347
assert_eq!(
308348
command_return.return_variant(),
309349
return_variant::SUCCESS_U32_U64

‎platform/src/error_code.rs

+2-6
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
11
/// An error code returned by the kernel.
2-
// TODO: derive(Debug) is currently only enabled for test builds, which is
3-
// necessary so it can be used in assert_eq!. We should develop a lighter-weight
4-
// Debug implementation and see if it is small enough to enable on non-Debug
5-
// builds.
6-
#[cfg_attr(test, derive(Debug))]
7-
#[derive(Clone, Copy, PartialEq, Eq)]
2+
// TODO: Add a ufmt debug implementation for process binaries to use.
3+
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
84
#[repr(u16)] // To facilitate use with transmute() in CommandReturn
95
#[rustfmt::skip]
106
pub enum ErrorCode {

‎platform/src/return_variant.rs

+2-6
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,7 @@
11
/// `ReturnVariant` describes what value type the kernel has returned.
22
// ReturnVariant is not an enum so that it can be converted from a u32 for free.
3-
// TODO: derive(Debug) is currently only enabled for test builds, which is
4-
// necessary so it can be used in assert_eq!. We should develop a lighter-weight
5-
// Debug implementation and see if it is small enough to enable on non-Debug
6-
// builds.
7-
#[cfg_attr(test, derive(Debug))]
8-
#[derive(Clone, Copy, PartialEq, Eq)]
3+
// TODO: Add a ufmt debug implementation for use by process binaries.
4+
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
95
pub struct ReturnVariant(u32);
106

117
impl From<u32> for ReturnVariant {

0 commit comments

Comments
 (0)