Not an in-depth review, just an explanation:
On Tue, Jul 18, 2023 at 09:19:54AM +0200, Lukas Wagner wrote:
> Thanks for the review!
>
> On 7/17/23 17:48, Maximiliano Sandoval wrote:
> >
> > Lukas Wagner <l.wag...@proxmox.com> writes:
> >
(...)
> > > +#[derive(Debug)]
> > > +pub enum Error {
> > > + ConfigSerialization(Box<dyn StdError + Send + Sync + 'static>),
FYI you can leave out the `'static` in type definitions like this. It's
the only possible choice and therefor automatic ;-)
^ Here we have `Box<dyn StdError>`
...
> > > + ConfigDeserialization(Box<dyn StdError + Send + Sync + 'static>),
> > > + NotifyFailed(String, Box<dyn StdError + Send + Sync + 'static>),
> > > + TargetDoesNotExist(String),
> > > +}
> > > +
> > > +impl Display for Error {
> > > + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
> > > + match self {
> > > + Error::ConfigSerialization(err) => {
> > > + write!(f, "could not serialize configuration: {err}")
> > > + }
> > > + Error::ConfigDeserialization(err) => {
> > > + write!(f, "could not deserialize configuration: {err}")
> > > + }
> > > + Error::NotifyFailed(endpoint, err) => {
> > > + write!(f, "could not notify via endpoint(s): {endpoint}:
> > > {err}")
> > > + }
> > > + Error::TargetDoesNotExist(target) => {
> > > + write!(f, "notification target '{target}' does not
> > > exist")
> > > + }
> > > + }
> > > + }
> > > +}
> > > +
> > > +impl StdError for Error {
> > > + fn source(&self) -> Option<&(dyn StdError + 'static)> {
> > > + match self {
^ Here, `self` is `&Self`
> > > + Error::ConfigSerialization(err) => Some(&**err),
> >
> > Does this really need the double deref?
>
> Yeah, does not seem to work any way. Though I'm not sure I can fully explain
> why.
> Copied the approach from (I think) the TFA crate.
Since, as noted above, `self` is `&Self`, `err` here will be `&T`.
`T` is `Box<dyn StdError + Send + Sync>`.
So we have `&Box<dyn StdError + Send + Sync>`
The first deref goes to `Box<dyn StdError + Send + Sync>` via an immutable ref.
We cannot return this as this would be a moving expression and rust does
not implicitly dereference multiple times automatically.
The second deref calls `<Box as Deref>::deref` and implicitly
dereferences it (since 'Deref' is kind of "transparent" to the '*'
operator), from `&(dyn StdError + Send + Sync)` to `(dyn StdError + Send
+ Sync)`.
Finally we borrow -> `&**` :-)
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
