Skip to content

Commit fc15f1f

Browse files
committed
add to_owned() methods to Proplist and format::Info
this has proven necessary for adding a lifetime annotation to the `Proplist` struct and consequently also to `format::Info`. (to follow if worthwhile). previously the `to_owned()` methods on introspection structs were creating owned copies of these two types via their `Clone` implementations (which always return owned copies). when experimenting with adding the mentioned lifetime annotations (including changing the output of the `Clone` implementations to `Foo<'static>`), i ran into confusing compiler errors complaining that the lifetime of the objects created (in the introspection `to_owned()` methods) might not live for the `'static` output lifetime. there was no error to suggest that my modified `Clone` implementations were invalid, so i couldn't understand what the problem was. i ended up creating `to_owned()` methods on the `Proplist` and `format::Info` structs, containing identical code to their `Clone` implementations, and tried using these methods instead. this fixed the errors. so it seems that for some reason the `Clone` implementations were not actually honouring the`'static` output lifetime I'd expressed. why? the `Clone` trait itself has no lifetime constraints, and does not provide an associated type for specifying the output of `clone()` which explicitly specifies `Self` as the output. so either you're not actually allowed to override the `Self` output like I did, in which case the compiler let me down by not pointing to the true source of the error; or the compiler mistakenly derived the lifetime from the `Self` expressed in the trait, rather the output expression in my implementation. interestingly (since lifetimes are somewhat poorly explained in rust documentation), from experimentation i've discovered that `Self` does not take lifetime(s) from function inputs, but rather from the `impl` line. thus in my commit where I'd added the lifetime to `Proplist`, if the compiler was indeed using the `Self` output expressed in the trait definition, then that `Self` would have taken the lifetime associated with the `Proplist` instance refered to in `&self`, which would then make sense why the lifetime would not have been long enough. but still, the compiler either has a bug picking the correct lifetime, or a deficiency in pointing out the true source of the error. additionally, the fact that my `Clone` implementations for these structs always output owned instances is not necessarily inline with proper expectations of cloning. note that `std::borrow::Cow` can hold both borrowed and owned types, and its clone behaviour is such that it creates a duplicate that matches the original, thus borrowed if the original is borrowed, or owned if the original is owned. mine should probably have the same behaviour. if i do change mine accordingly, I'd need such `to_owned()` methods as added here to provide a means of forcing the creation of an owned instance from a borrowed one. note that the return type of `Self` here is fine since the types do not currently have a lifetime annotation, so effectively the return instances are owned/static.
1 parent 39c8464 commit fc15f1f

File tree

4 files changed

+33
-18
lines changed

4 files changed

+33
-18
lines changed

pulse-binding/src/context/ext_device_restore.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ impl Info {
5656
/// Creates a copy with owned data.
5757
pub fn to_owned(&self) -> Self {
5858
Info {
59-
formats: self.formats.clone(), //Our implementation makes an owned copy!
59+
formats: self.formats.iter().map(format::Info::to_owned).collect(),
6060
..*self
6161
}
6262
}

pulse-binding/src/context/introspect.rs

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -471,10 +471,10 @@ impl SinkInfo<'_> {
471471
description: self.description.clone().map(|o| Cow::Owned(o.into_owned())),
472472
monitor_source_name: self.monitor_source_name.clone().map(|o| Cow::Owned(o.into_owned())),
473473
driver: self.driver.clone().map(|o| Cow::Owned(o.into_owned())),
474-
proplist: self.proplist.clone(), //Our implementation makes an owned copy!
474+
proplist: self.proplist.to_owned(),
475475
ports: self.ports.iter().map(SinkPortInfo::to_owned).collect(),
476476
active_port: self.active_port.as_ref().map(|ap| Box::new(ap.as_ref().to_owned())),
477-
formats: self.formats.clone(), //Our implementation makes an owned copy!
477+
formats: self.formats.iter().map(format::Info::to_owned).collect(),
478478
..*self
479479
}
480480
}
@@ -901,10 +901,10 @@ impl SourceInfo<'_> {
901901
description: self.description.clone().map(|o| Cow::Owned(o.into_owned())),
902902
monitor_of_sink_name: self.monitor_of_sink_name.clone().map(|o| Cow::Owned(o.into_owned())),
903903
driver: self.driver.clone().map(|o| Cow::Owned(o.into_owned())),
904-
proplist: self.proplist.clone(), //Our implementation makes an owned copy!
904+
proplist: self.proplist.to_owned(),
905905
ports: self.ports.iter().map(SourcePortInfo::to_owned).collect(),
906906
active_port: self.active_port.as_ref().map(|ap| Box::new(ap.as_ref().to_owned())),
907-
formats: self.formats.clone(), //Our implementation makes an owned copy!
907+
formats: self.formats.iter().map(format::Info::to_owned).collect(),
908908
..*self
909909
}
910910
}
@@ -1271,7 +1271,7 @@ impl ModuleInfo<'_> {
12711271
ModuleInfo {
12721272
name: self.name.clone().map(|o| Cow::Owned(o.into_owned())),
12731273
argument: self.argument.clone().map(|o| Cow::Owned(o.into_owned())),
1274-
proplist: self.proplist.clone(), //Our implementation makes an owned copy!
1274+
proplist: self.proplist.to_owned(),
12751275
..*self
12761276
}
12771277
}
@@ -1469,7 +1469,7 @@ impl ClientInfo<'_> {
14691469
ClientInfo {
14701470
name: self.name.clone().map(|o| Cow::Owned(o.into_owned())),
14711471
driver: self.driver.clone().map(|o| Cow::Owned(o.into_owned())),
1472-
proplist: self.proplist.clone(), //Our implementation makes an owned copy!
1472+
proplist: self.proplist.to_owned(),
14731473
..*self
14741474
}
14751475
}
@@ -1688,7 +1688,7 @@ impl CardPortInfo<'_> {
16881688
CardPortInfo {
16891689
name: self.name.clone().map(|o| Cow::Owned(o.into_owned())),
16901690
description: self.description.clone().map(|o| Cow::Owned(o.into_owned())),
1691-
proplist: self.proplist.clone(), //Our implementation makes an owned copy!
1691+
proplist: self.proplist.to_owned(),
16921692
profiles: self.profiles.iter().map(CardProfileInfo::to_owned).collect(),
16931693
#[cfg(any(doc, feature = "pa_v14"))]
16941694
availability_group: self.availability_group.clone().map(|o| Cow::Owned(o.into_owned())),
@@ -1776,7 +1776,7 @@ impl CardInfo<'_> {
17761776
CardInfo {
17771777
name: self.name.clone().map(|o| Cow::Owned(o.into_owned())),
17781778
driver: self.driver.clone().map(|o| Cow::Owned(o.into_owned())),
1779-
proplist: self.proplist.clone(), //Our implementation makes an owned copy!
1779+
proplist: self.proplist.to_owned(),
17801780
ports: self.ports.iter().map(CardPortInfo::to_owned).collect(),
17811781
profiles: self.profiles.iter().map(CardProfileInfo::to_owned).collect(),
17821782
active_profile: self.active_profile.as_ref().map(|ap| Box::new(ap.as_ref().to_owned())),
@@ -2014,8 +2014,8 @@ impl SinkInputInfo<'_> {
20142014
name: self.name.clone().map(|o| Cow::Owned(o.into_owned())),
20152015
resample_method: self.resample_method.clone().map(|o| Cow::Owned(o.into_owned())),
20162016
driver: self.driver.clone().map(|o| Cow::Owned(o.into_owned())),
2017-
proplist: self.proplist.clone(), //Our implementation makes an owned copy!
2018-
format: self.format.clone(), //Our implementation makes an owned copy!
2017+
proplist: self.proplist.to_owned(),
2018+
format: self.format.to_owned(),
20192019
..*self
20202020
}
20212021
}
@@ -2255,8 +2255,8 @@ impl SourceOutputInfo<'_> {
22552255
name: self.name.clone().map(|o| Cow::Owned(o.into_owned())),
22562256
resample_method: self.resample_method.clone().map(|o| Cow::Owned(o.into_owned())),
22572257
driver: self.driver.clone().map(|o| Cow::Owned(o.into_owned())),
2258-
proplist: self.proplist.clone(), //Our implementation makes an owned copy!
2259-
format: self.format.clone(), //Our implementation makes an owned copy!
2258+
proplist: self.proplist.to_owned(),
2259+
format: self.format.to_owned(),
22602260
..*self
22612261
}
22622262
}
@@ -2479,7 +2479,7 @@ impl SampleInfo<'_> {
24792479
SampleInfo {
24802480
name: self.name.clone().map(|o| Cow::Owned(o.into_owned())),
24812481
filename: self.filename.clone().map(|o| Cow::Owned(o.into_owned())),
2482-
proplist: self.proplist.clone(), //Our implementation makes an owned copy!
2482+
proplist: self.proplist.to_owned(),
24832483
..*self
24842484
}
24852485
}

pulse-binding/src/format.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,15 @@ impl Info {
224224
unsafe { Self { ptr: ptr, properties: Proplist::from_raw_weak((*ptr).list), weak: true } }
225225
}
226226

227+
/// Returns a new `Info` struct representing the same format.
228+
///
229+
/// If this is called on a ‘weak’ instance, a non-weak object is returned.
230+
#[inline]
231+
pub(crate) fn to_owned(&self) -> Self {
232+
let ptr = unsafe { capi::pa_format_info_copy(self.ptr as *const capi::pa_format_info) };
233+
Self::from_raw(ptr as *mut InfoInternal)
234+
}
235+
227236
/// Checks whether the `Info` structure is valid.
228237
#[inline]
229238
pub fn is_valid(&self) -> bool {
@@ -592,8 +601,6 @@ impl Clone for Info {
592601
///
593602
/// If this is called on a ‘weak’ instance, a non-weak object is returned.
594603
fn clone(&self) -> Self {
595-
let ptr = unsafe { capi::pa_format_info_copy(self.ptr as *const capi::pa_format_info) };
596-
assert_eq!(false, ptr.is_null());
597-
Self::from_raw(ptr as *mut InfoInternal)
604+
self.to_owned()
598605
}
599606
}

pulse-binding/src/proplist.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,14 @@ impl Proplist {
251251
Proplist(ProplistInner { ptr: ptr, weak: true })
252252
}
253253

254+
/// Allocates a new property list and copies over every single entry from the specified list.
255+
///
256+
/// If this is called on a ‘weak’ instance, a non-weak object is returned.
257+
#[inline]
258+
pub(crate) fn to_owned(&self) -> Self {
259+
Self::from_raw(unsafe { capi::pa_proplist_copy(self.0.ptr) })
260+
}
261+
254262
/// Checks if the key is valid.
255263
pub fn key_is_valid(key: &str) -> bool {
256264
// Warning: New CStrings will be immediately freed if not bound to a variable, leading to
@@ -499,7 +507,7 @@ impl Clone for Proplist {
499507
/// If this is called on a ‘weak’ instance, a non-weak object is returned.
500508
#[inline]
501509
fn clone(&self) -> Self {
502-
Self::from_raw(unsafe { capi::pa_proplist_copy(self.0.ptr) })
510+
self.to_owned()
503511
}
504512
}
505513

0 commit comments

Comments
 (0)