Skip to content

Commit c46d181

Browse files
jonathanpallantjonathanpallant
committed
Remove .expect() when parsing USB requests
Resolves issue with macOS now sending requests we cannot parse. It's fine to just stall them - no need to crash the CPU because of it.
1 parent bd92963 commit c46d181

File tree

7 files changed

+94
-67
lines changed

7 files changed

+94
-67
lines changed

‎nrf52-code/usb-app-solutions/src/bin/usb-2.rs‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -85,19 +85,18 @@ fn on_event(usbd: &Usbd, event: Event) {
8585
wvalue
8686
);
8787

88-
let request = Request::parse(bmrequesttype, brequest, wvalue, windex, wlength)
89-
.expect("Error parsing request");
88+
let request = Request::parse(bmrequesttype, brequest, wvalue, windex, wlength);
9089
match request {
91-
Request::GetDescriptor {
90+
Ok(Request::GetDescriptor {
9291
descriptor: Descriptor::Device,
9392
length,
94-
} => {
93+
}) => {
9594
defmt::info!("GET_DESCRIPTOR Device [length={}]", length);
9695

9796
defmt::println!("Goal reached; move to the next section");
9897
dk::exit()
9998
}
100-
Request::SetAddress { .. } => {
99+
Ok(Request::SetAddress { .. }) => {
101100
// On macOS you'll get this request before the GET_DESCRIPTOR request so we
102101
// need to catch it here. We'll properly handle this request later
103102
// but for now it's OK to do nothing.

‎nrf52-code/usb-app-solutions/src/bin/usb-3.rs‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -73,14 +73,12 @@ fn on_event(usbd: &Usbd, ep0in: &mut Ep0In, event: Event) {
7373
wvalue
7474
);
7575

76-
let request = Request::parse(bmrequesttype, brequest, wvalue, windex, wlength).expect(
77-
"Error parsing request (goal achieved if GET_DESCRIPTOR Device was handled before)",
78-
);
76+
let request = Request::parse(bmrequesttype, brequest, wvalue, windex, wlength);
7977
match request {
80-
Request::GetDescriptor {
78+
Ok(Request::GetDescriptor {
8179
descriptor: Descriptor::Device,
8280
length,
83-
} => {
81+
}) => {
8482
defmt::info!("GET_DESCRIPTOR Device [length={}]", length);
8583

8684
let desc = usb2::device::Descriptor {
@@ -107,15 +105,15 @@ fn on_event(usbd: &Usbd, ep0in: &mut Ep0In, event: Event) {
107105
};
108106
ep0in.start(subslice, usbd);
109107
}
110-
Request::SetAddress { .. } => {
108+
Ok(Request::SetAddress { .. }) => {
111109
// On macOS you'll get this request before the GET_DESCRIPTOR request so we
112110
// need to catch it here. We'll properly handle this request later
113111
// but for now it's OK to do nothing.
114112
}
115113
_ => {
116114
defmt::error!(
117-
"unknown request (goal achieved if GET_DESCRIPTOR Device was handled before)"
118-
);
115+
"unknown request (goal achieved if GET_DESCRIPTOR Device was handled before)"
116+
);
119117
dk::exit()
120118
}
121119
}

‎nrf52-code/usb-app-solutions/src/bin/usb-4.rs‎

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -94,18 +94,17 @@ fn ep0setup(usbd: &Usbd, ep0in: &mut Ep0In, state: &mut State) -> Result<(), ()>
9494
wvalue
9595
);
9696

97-
let request = Request::parse(bmrequesttype, brequest, wvalue, windex, wlength)
98-
.expect("Error parsing request");
97+
let request = Request::parse(bmrequesttype, brequest, wvalue, windex, wlength);
9998
defmt::info!("EP0: {}", defmt::Debug2Format(&request));
100-
// ^^^^^^^^^^^^^^^^^^^ this adapter is currently needed to log
101-
// `Request` with `defmt`
99+
// ^^^^^^^^^^^^^^^^^^^ this adapter is currently needed to log
100+
// `Request` with `defmt`
102101
match request {
103102
// section 9.4.3
104103
// this request is valid in any state
105-
Request::GetDescriptor {
104+
Ok(Request::GetDescriptor {
106105
descriptor: Descriptor::Device,
107106
length,
108-
} => {
107+
}) => {
109108
let desc = usb2::device::Descriptor {
110109
bDeviceClass: 0,
111110
bDeviceProtocol: 0,
@@ -130,10 +129,10 @@ fn ep0setup(usbd: &Usbd, ep0in: &mut Ep0In, state: &mut State) -> Result<(), ()>
130129
};
131130
ep0in.start(subslice, usbd);
132131
}
133-
Request::GetDescriptor {
132+
Ok(Request::GetDescriptor {
134133
descriptor: Descriptor::Configuration { index },
135134
length,
136-
} => {
135+
}) => {
137136
if index == 0 {
138137
let mut resp = heapless::Vec::<u8, 64>::new();
139138

@@ -169,7 +168,9 @@ fn ep0setup(usbd: &Usbd, ep0in: &mut Ep0In, state: &mut State) -> Result<(), ()>
169168
return Err(());
170169
}
171170
}
172-
Request::SetAddress { address } => {
171+
Ok(Request::SetAddress { address }) => {
172+
// On macOS you'll get this request before the GET_DESCRIPTOR request so we
173+
// need to catch it here.
173174
match state {
174175
State::Default => {
175176
if let Some(address) = address {
@@ -194,9 +195,21 @@ fn ep0setup(usbd: &Usbd, ep0in: &mut Ep0In, state: &mut State) -> Result<(), ()>
194195

195196
// the response to this request is handled in hardware
196197
}
197-
198-
// stall any other request
199-
_ => return Err(()),
198+
Ok(_) => {
199+
// stall anything else
200+
return Err(());
201+
}
202+
Err(_) => {
203+
defmt::error!(
204+
"Failed to parse: bmrequesttype: 0b{=u8:08b}, brequest: {=u8}, wlength: {=u16}, windex: 0x{=u16:04x}, wvalue: 0x{=u16:04x}",
205+
bmrequesttype,
206+
brequest,
207+
wlength,
208+
windex,
209+
wvalue
210+
);
211+
return Err(());
212+
}
200213
}
201214

202215
Ok(())

‎nrf52-code/usb-app-solutions/src/bin/usb-5.rs‎

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -94,19 +94,18 @@ fn ep0setup(usbd: &Usbd, ep0in: &mut Ep0In, state: &mut State) -> Result<(), ()>
9494
wvalue
9595
);
9696

97-
let request = Request::parse(bmrequesttype, brequest, wvalue, windex, wlength)
98-
.expect("Error parsing request");
97+
let request = Request::parse(bmrequesttype, brequest, wvalue, windex, wlength);
9998
defmt::info!("EP0: {}", defmt::Debug2Format(&request));
10099
// ^^^^^^^^^^^^^^^^^^^ this adapter is currently needed to log
101100
// `Request` with `defmt`
102101

103102
match request {
104103
// section 9.4.3
105104
// this request is valid in any state
106-
Request::GetDescriptor {
105+
Ok(Request::GetDescriptor {
107106
descriptor: Descriptor::Device,
108107
length,
109-
} => {
108+
}) => {
110109
let desc = usb2::device::Descriptor {
111110
bDeviceClass: 0,
112111
bDeviceProtocol: 0,
@@ -131,10 +130,10 @@ fn ep0setup(usbd: &Usbd, ep0in: &mut Ep0In, state: &mut State) -> Result<(), ()>
131130
};
132131
ep0in.start(subslice, usbd);
133132
}
134-
Request::GetDescriptor {
133+
Ok(Request::GetDescriptor {
135134
descriptor: Descriptor::Configuration { index },
136135
length,
137-
} => {
136+
}) => {
138137
if index == 0 {
139138
let mut resp = heapless::Vec::<u8, 64>::new();
140139

@@ -170,7 +169,7 @@ fn ep0setup(usbd: &Usbd, ep0in: &mut Ep0In, state: &mut State) -> Result<(), ()>
170169
return Err(());
171170
}
172171
}
173-
Request::SetAddress { address } => {
172+
Ok(Request::SetAddress { address }) => {
174173
match state {
175174
State::Default => {
176175
if let Some(address) = address {
@@ -195,7 +194,7 @@ fn ep0setup(usbd: &Usbd, ep0in: &mut Ep0In, state: &mut State) -> Result<(), ()>
195194

196195
// the response to this request is handled in hardware
197196
}
198-
Request::SetConfiguration { value } => {
197+
Ok(Request::SetConfiguration { value }) => {
199198
match *state {
200199
// unspecified behavior
201200
State::Default => return Err(()),
@@ -240,9 +239,21 @@ fn ep0setup(usbd: &Usbd, ep0in: &mut Ep0In, state: &mut State) -> Result<(), ()>
240239

241240
usbd.tasks_ep0status().write_value(1);
242241
}
243-
244-
// stall any other request
245-
_ => return Err(()),
242+
Ok(_) => {
243+
// stall anything else
244+
return Err(());
245+
}
246+
Err(_) => {
247+
defmt::error!(
248+
"Failed to parse: bmrequesttype: 0b{=u8:08b}, brequest: {=u8}, wlength: {=u16}, windex: 0x{=u16:04x}, wvalue: 0x{=u16:04x}",
249+
bmrequesttype,
250+
brequest,
251+
wlength,
252+
windex,
253+
wvalue
254+
);
255+
return Err(());
256+
}
246257
}
247258

248259
Ok(())

‎nrf52-code/usb-app/src/bin/usb-2.rs‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,12 @@ fn on_event(_usbd: &Usbd, event: Event) {
7777
wvalue
7878
);
7979

80-
let request = Request::parse(bmrequesttype, brequest, wvalue, windex, wlength)
81-
.expect("Error parsing request");
80+
let request = Request::parse(bmrequesttype, brequest, wvalue, windex, wlength);
8281
match request {
83-
Request::GetDescriptor {
82+
Ok(Request::GetDescriptor {
8483
descriptor: Descriptor::Device,
8584
length,
86-
} => {
85+
}) => {
8786
// TODO modify `Request::parse()` in `nrf52-code/usb-lib/src/lib.rs`
8887
// so that this branch is reached
8988

@@ -92,12 +91,11 @@ fn on_event(_usbd: &Usbd, event: Event) {
9291
defmt::println!("Goal reached; move to the next section");
9392
dk::exit()
9493
}
95-
Request::SetAddress { .. } => {
94+
Ok(Request::SetAddress { .. }) => {
9695
// On macOS you'll get this request before the GET_DESCRIPTOR request so we
9796
// need to catch it here. We'll properly handle this request later
9897
// but for now it's OK to do nothing.
9998
}
100-
#[allow(unreachable_patterns)]
10199
_ => unreachable!(), // we don't handle any other Requests
102100
}
103101
}

‎nrf52-code/usb-app/src/bin/usb-3.rs‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -73,31 +73,28 @@ fn on_event(usbd: &Usbd, ep0in: &mut Ep0In, event: Event) {
7373
wvalue
7474
);
7575

76-
let request = Request::parse(bmrequesttype, brequest, wvalue, windex, wlength).expect(
77-
"Error parsing request (goal achieved if GET_DESCRIPTOR Device was handled before)",
78-
);
76+
let request = Request::parse(bmrequesttype, brequest, wvalue, windex, wlength);
7977
match request {
80-
Request::GetDescriptor {
78+
Ok(Request::GetDescriptor {
8179
descriptor: Descriptor::Device,
8280
length,
83-
} => {
81+
}) => {
8482
defmt::info!("GET_DESCRIPTOR Device [length={}]", length);
8583

8684
// TODO send back a valid device descriptor, truncated to `length` bytes
8785
// let desc = usb2::device::Descriptor { .. };
8886
let resp = [];
8987
ep0in.start(&resp, usbd);
9088
}
91-
Request::SetAddress { .. } => {
89+
Ok(Request::SetAddress { .. }) => {
9290
// On macOS you'll get this request before the GET_DESCRIPTOR request so we
9391
// need to catch it here. We'll properly handle this request later
9492
// but for now it's OK to do nothing.
9593
}
96-
#[allow(unreachable_patterns)]
9794
_ => {
9895
defmt::error!(
99-
"unknown request (goal achieved if GET_DESCRIPTOR Device was handled before)"
100-
);
96+
"unknown request (goal achieved if GET_DESCRIPTOR Device was handled before)"
97+
);
10198
dk::exit()
10299
}
103100
}

‎nrf52-code/usb-app/src/bin/usb-4.rs‎

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -95,17 +95,15 @@ fn ep0setup(usbd: &Usbd, ep0in: &mut Ep0In, _state: &mut State) -> Result<(), ()
9595
wvalue
9696
);
9797

98-
let request = Request::parse(bmrequesttype, brequest, wvalue, windex, wlength)
99-
.expect("Error parsing request");
98+
let request = Request::parse(bmrequesttype, brequest, wvalue, windex, wlength);
10099
defmt::info!("EP0: {}", defmt::Debug2Format(&request));
101-
// ^^^^^^^^^^^^^^^^^^^ this adapter is currently needed to log
102-
// `StandardRequest` with `defmt`
103-
100+
// ^^^^^^^^^^^^^^^^^^^ this adapter is currently needed to log
101+
// `Request` with `defmt`
104102
match request {
105-
Request::GetDescriptor {
103+
Ok(Request::GetDescriptor {
106104
descriptor: Descriptor::Device,
107105
length,
108-
} => {
106+
}) => {
109107
let desc = usb2::device::Descriptor {
110108
bDeviceClass: 0,
111109
bDeviceProtocol: 0,
@@ -120,34 +118,47 @@ fn ep0setup(usbd: &Usbd, ep0in: &mut Ep0In, _state: &mut State) -> Result<(), ()
120118
idVendor: consts::USB_VID_DEMO,
121119
};
122120
let length = usize::from(length);
123-
let bytes = desc.bytes();
124-
let subslice = if length >= bytes.len() {
121+
let desc_bytes = desc.bytes();
122+
let subslice = if length >= desc_bytes.len() {
125123
// they want all of it
126-
&bytes[0..]
124+
&desc_bytes[0..]
127125
} else {
128126
// they don't want all of it
129-
&bytes[0..length]
127+
&desc_bytes[0..length]
130128
};
131129
ep0in.start(subslice, usbd);
132130
}
133131
// TODO implement Configuration descriptor
134-
Request::GetDescriptor {
132+
Ok(Request::GetDescriptor {
135133
// descriptor: Descriptor::Configuration { index },
136134
descriptor: _,
137135
length: _,
138-
} => {
136+
}) => {
139137
return Err(());
140138
}
141-
Request::SetAddress { .. } => {
139+
Ok(Request::SetAddress { .. }) => {
142140
// On macOS you'll get this request before the GET_DESCRIPTOR request so we
143141
// need to catch it here.
144142

145143
// TODO: handle this request properly now.
146144
todo!()
147145
}
148146

149-
// stall any other request
150-
_ => return Err(()),
147+
Ok(_) => {
148+
// stall anything else
149+
return Err(());
150+
}
151+
Err(_) => {
152+
defmt::error!(
153+
"Failed to parse: bmrequesttype: 0b{=u8:08b}, brequest: {=u8}, wlength: {=u16}, windex: 0x{=u16:04x}, wvalue: 0x{=u16:04x}",
154+
bmrequesttype,
155+
brequest,
156+
wlength,
157+
windex,
158+
wvalue
159+
);
160+
return Err(());
161+
}
151162
}
152163

153164
Ok(())

0 commit comments

Comments
 (0)