Re: [pbs-devel] [PATCH proxmox v4 3/4] proxmox-async: add connect_to_udp helper

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

On Tue, Feb 01, 2022 at 01:39:00PM +0100, Thomas Lamprecht wrote:
> On 01.02.22 13:13, Dominik Csapak wrote:
> > On 2/1/22 13:02, Thomas Lamprecht wrote:
> >>> +    addr: A,
> >>> +) -> io::Result<UdpSocket> {
> >>> +    let socket = match tokio::net::lookup_host(&addr).await?.next() {> +        Some(SocketAddr::V4(_)) => UdpSocket::bind("0.0.0.0:0").await?,
> >>> +        Some(SocketAddr::V6(_)) => UdpSocket::bind("[::]:0").await?,
> >>> +        None => proxmox_sys::io_bail!("could not resolve address family {}", addr),
> >>
> >> would it have some merit to use {:?} to loose the Display trait bound?
> >> Probably not to relevant though.
> >>
> > 
> > then we'd need the Debug trait though, so no real gain?
> > 
> 
> hmm, thought that was implied, meh.. Normally one would want to pass it via the error
> (type) upwards and let the caller handle it, but those error types can be stupid too...

Just don't include it in the error and drop the additional trait
requirement as this is supposed to provide what should be a standard
function which wouldn't do that either. It should look & feel more like
`std::net::TcpSocket::connect` / `tokio::net::TcpSocket::connect` already
do.

Also, please actually really loop over the addresses instead of just
using the first one.
Note that in `std` the error for not finding any address is currently[1]:

    Error::new(ErrorKind::InvalidInput, &"could not resolve to any addresses")

and while I have a strong aversion against abusing system error types
for something that doesn't actually come from a syscall (EINVAL in this
case), in thise case we can even do the same since this should be part
of std anyway...

[1] https://github.com/rust-lang/rust/blob/74fbbefea8d13683cca5eee62e4740706cb3144a/library/std/src/net/mod.rs#L77