base: improve documentation of Descriptor and AsRawDescriptor

Although it can be useful for some limited cases, the Descriptor wrapper
class should ideally be eschewed and an alternative based on
implementing AsRawDescriptor on the right type be preferred. Make sure
its documentation reflects that.

The reason for avoiding Descriptor is that it encourages storing raw
descriptors that can possibly be closed before the user expects it. A
reasonable alternative to doing this is to call as_raw_descriptor() at
the exact moment the descriptor is needed, and never storing the result
- this ensures that the providing object (and underlying descriptor)
remain alive for long enough. Update the documentation of
AsRawDescriptor to explain and hopefully encourage this practice.

BUG=233968702
TEST=cargo build

Change-Id: I91c2ef11152b3b6adcc25f40d6de0a0f131700dd
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3670101
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
This commit is contained in:
Alexandre Courbot 2022-05-26 11:54:00 +09:00 committed by Chromeos LUCI
parent ed66850d92
commit aa2e59d82f

View file

@ -25,11 +25,26 @@ pub trait IntoRawDescriptor {
/// Trait for returning the underlying raw descriptor, without giving up ownership of the
/// descriptor.
pub trait AsRawDescriptor {
/// Returns the underlying raw descriptor.
///
/// Since the descriptor is still owned by the provider, callers should not assume that it will
/// remain open for longer than the immediate call of this method. In particular, it is a
/// dangerous practice to store the result of this method for future use: instead, it should be
/// used to e.g. obtain a raw descriptor that is immediately passed to a system call.
///
/// If you need to use the descriptor for a longer time (and particularly if you cannot reliably
/// track the lifetime of the providing object), you should probably consider using
/// [`SafeDescriptor`] (possibly along with [`trait@IntoRawDescriptor`]) to get full ownership
/// over a descriptor pointing to the same resource.
fn as_raw_descriptor(&self) -> RawDescriptor;
}
/// A trait similar to `AsRawDescriptor` but supports an arbitrary number of descriptors.
pub trait AsRawDescriptors {
/// Returns the underlying raw descriptors.
///
/// Please refer to the documentation of [`AsRawDescriptor::as_raw_descriptor`] for limitations
/// and recommended use.
fn as_raw_descriptors(&self) -> Vec<RawDescriptor>;
}
@ -106,9 +121,26 @@ impl From<File> for SafeDescriptor {
}
}
/// For use cases where a simple wrapper around a RawDescriptor is needed.
/// This is a simply a wrapper and does not manage the lifetime of the descriptor.
/// Most usages should prefer SafeDescriptor or using a RawDescriptor directly
/// For use cases where a simple wrapper around a [`RawDescriptor`] is needed, in order to e.g.
/// implement [`trait@AsRawDescriptor`].
///
/// This is a simply a wrapper and does not manage the lifetime of the descriptor. As such it is the
/// responsibility of the user to ensure that the wrapped descriptor will not be closed for as long
/// as the `Descriptor` is alive.
///
/// Most use-cases should prefer [`SafeDescriptor`] or implementing and using
/// [`trait@AsRawDescriptor`] on the type providing the descriptor. Using this wrapper usually means
/// something can be improved in your code.
///
/// Valid uses of this struct include:
/// * You only have a valid [`RawDescriptor`] and need to pass something that implements
/// [`trait@AsRawDescriptor`] to a function,
/// * You need to serialize a [`RawDescriptor`],
/// * You need [`trait@Send`] or [`trait@Sync`] for your descriptor and properly handle the case
/// where your descriptor gets closed.
///
/// Note that with the exception of the last use-case (which requires proper error checking against
/// the descriptor being closed), the `Descriptor` instance would be very short-lived.
#[derive(Copy, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)]
#[repr(transparent)]
pub struct Descriptor(pub RawDescriptor);