From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id C3872E404 for ; Tue, 18 Jul 2023 12:13:47 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 9F8EB1969A for ; Tue, 18 Jul 2023 12:13:17 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Tue, 18 Jul 2023 12:13:17 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id B8ED843015 for ; Tue, 18 Jul 2023 12:13:16 +0200 (CEST) Date: Tue, 18 Jul 2023 12:13:15 +0200 From: Wolfgang Bumiller To: Lukas Wagner Cc: Maximiliano Sandoval , Proxmox VE development discussion Message-ID: References: <20230717150051.710464-1-l.wagner@proxmox.com> <20230717150051.710464-3-l.wagner@proxmox.com> <4844941f-43e0-121c-0038-4fe577f4da98@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4844941f-43e0-121c-0038-4fe577f4da98@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.120 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - Subject: Re: [pve-devel] [PATCH v3 proxmox 02/66] notify: preparation for the first endpoint plugin X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 18 Jul 2023 10:13:47 -0000 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 writes: > > (...) > > > +#[derive(Debug)] > > > +pub enum Error { > > > + ConfigSerialization(Box), 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` ... > > > + ConfigDeserialization(Box), > > > + NotifyFailed(String, Box), > > > + 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`. So we have `&Box` The first deref goes to `Box` 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 `::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 -> `&**` :-)