Skip to content

Commit 33ffb0a

Browse files
tamirdvireshk
authored andcommitted
rust: opp: simplify callers of to_c_str_array
Use `Option` combinators to make this a bit less noisy. Wrap the `dev_pm_opp_set_config` operation in a closure and use type ascription to leverage the compiler to check for use after free. Signed-off-by: Tamir Duberstein <tamird@kernel.org> Tested-by: Viresh Kumar <viresh.kumar@linaro.org> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
1 parent 173e02d commit 33ffb0a

1 file changed

Lines changed: 58 additions & 54 deletions

File tree

rust/kernel/opp.rs

Lines changed: 58 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -443,66 +443,70 @@ impl<T: ConfigOps + Default> Config<T> {
443443
///
444444
/// The returned [`ConfigToken`] will remove the configuration when dropped.
445445
pub fn set(self, dev: &Device) -> Result<ConfigToken> {
446-
let (_clk_list, clk_names) = match &self.clk_names {
447-
Some(x) => {
448-
let list = to_c_str_array(x)?;
449-
let ptr = list.as_ptr();
450-
(Some(list), ptr)
451-
}
452-
None => (None, ptr::null()),
453-
};
446+
let clk_names = self.clk_names.as_deref().map(to_c_str_array).transpose()?;
447+
let regulator_names = self
448+
.regulator_names
449+
.as_deref()
450+
.map(to_c_str_array)
451+
.transpose()?;
452+
453+
let set_config = || {
454+
let clk_names = clk_names.as_ref().map_or(ptr::null(), |c| c.as_ptr());
455+
let regulator_names = regulator_names.as_ref().map_or(ptr::null(), |c| c.as_ptr());
456+
457+
let prop_name = self
458+
.prop_name
459+
.as_ref()
460+
.map_or(ptr::null(), |p| p.as_char_ptr());
461+
462+
let (supported_hw, supported_hw_count) = self
463+
.supported_hw
464+
.as_ref()
465+
.map_or((ptr::null(), 0), |hw| (hw.as_ptr(), hw.len() as u32));
466+
467+
let (required_dev, required_dev_index) = self
468+
.required_dev
469+
.as_ref()
470+
.map_or((ptr::null_mut(), 0), |(dev, idx)| (dev.as_raw(), *idx));
471+
472+
let mut config = bindings::dev_pm_opp_config {
473+
clk_names,
474+
config_clks: if T::HAS_CONFIG_CLKS {
475+
Some(Self::config_clks)
476+
} else {
477+
None
478+
},
479+
prop_name,
480+
regulator_names,
481+
config_regulators: if T::HAS_CONFIG_REGULATORS {
482+
Some(Self::config_regulators)
483+
} else {
484+
None
485+
},
486+
supported_hw,
487+
supported_hw_count,
454488

455-
let (_regulator_list, regulator_names) = match &self.regulator_names {
456-
Some(x) => {
457-
let list = to_c_str_array(x)?;
458-
let ptr = list.as_ptr();
459-
(Some(list), ptr)
460-
}
461-
None => (None, ptr::null()),
462-
};
489+
required_dev,
490+
required_dev_index,
491+
};
463492

464-
let prop_name = self
465-
.prop_name
466-
.as_ref()
467-
.map_or(ptr::null(), |p| p.as_char_ptr());
468-
469-
let (supported_hw, supported_hw_count) = self
470-
.supported_hw
471-
.as_ref()
472-
.map_or((ptr::null(), 0), |hw| (hw.as_ptr(), hw.len() as u32));
473-
474-
let (required_dev, required_dev_index) = self
475-
.required_dev
476-
.as_ref()
477-
.map_or((ptr::null_mut(), 0), |(dev, idx)| (dev.as_raw(), *idx));
478-
479-
let mut config = bindings::dev_pm_opp_config {
480-
clk_names,
481-
config_clks: if T::HAS_CONFIG_CLKS {
482-
Some(Self::config_clks)
483-
} else {
484-
None
485-
},
486-
prop_name,
487-
regulator_names,
488-
config_regulators: if T::HAS_CONFIG_REGULATORS {
489-
Some(Self::config_regulators)
490-
} else {
491-
None
492-
},
493-
supported_hw,
494-
supported_hw_count,
493+
// SAFETY: The requirements are satisfied by the existence of [`Device`] and its safety
494+
// requirements. The OPP core guarantees not to access fields of [`Config`] after this
495+
// call and so we don't need to save a copy of them for future use.
496+
let ret = unsafe { bindings::dev_pm_opp_set_config(dev.as_raw(), &mut config) };
495497

496-
required_dev,
497-
required_dev_index,
498+
to_result(ret).map(|()| ConfigToken(ret))
498499
};
499500

500-
// SAFETY: The requirements are satisfied by the existence of [`Device`] and its safety
501-
// requirements. The OPP core guarantees not to access fields of [`Config`] after this call
502-
// and so we don't need to save a copy of them for future use.
503-
let ret = unsafe { bindings::dev_pm_opp_set_config(dev.as_raw(), &mut config) };
501+
// Ensure the closure does not accidentally drop owned data; if violated, the compiler
502+
// produces E0525 with e.g.:
503+
//
504+
// ```
505+
// closure is `FnOnce` because it moves the variable `clk_names` out of its environment
506+
// ```
507+
let _: &dyn Fn() -> _ = &set_config;
504508

505-
to_result(ret).map(|()| ConfigToken(ret))
509+
set_config()
506510
}
507511

508512
/// Config's clk callback.

0 commit comments

Comments
 (0)