From: Cole Helbling <cole.e.helbling@outlook.com>
To: Alyssa Ross <hi@alyssa.is>, Cole Helbling <cole.e.helbling@outlook.com>
Cc: devel@spectrum-os.org
Subject: Re: [PATCH crosvm 1/2] msg_socket: introduce UnixSeqpacketExt
Date: Mon, 22 Jun 2020 15:06:50 -0700 [thread overview]
Message-ID: <CH2PR14MB3579DD94ED9CDFE54ADAB43BB3970@CH2PR14MB3579.namprd14.prod.outlook.com> (raw)
In-Reply-To: <87a713qrto.fsf@alyssa.is>
Alyssa Ross <hi@alyssa.is> writes:
> Did you see the way it was used in the second patch? We put all the
> sockets into a Vec<&UnixSeqpacket>, ask a function which is the right
> socket to send the message to, and then send the message to that
> socket. It's not really possible to do that sort of thing while
> preserving type information, because you can't really have a collection
> containing members of different types.
Ah, yeah. That's pretty obvious now that you've explained it...
> --->8---
>
> Both 1 and 3 would provide full compile-time type safety, at the expense
> of being a bit more horrible than 2. I think I'm leaning towards 1, but
> I don't have strong feelings about any of them. Having thought about
> this now though, in response to your question (thanks!), I definitely
> think that we should do something here better than what I presented in
> these patches.
>
> I'd be very curious to hear your thoughts.
I think that options 1 or 3 are the best choices. My only qualm with
option 1 is that it uses a callback. I can't even really explain *why* I
don't like callbacks (maybe because I had a bad time trying to use them
within Rust before, when I was lacking experience), but it just feels
somehow wrong to me. (Maybe it's also because I stereotype callbacks to
C -- which isn't necessarily a bad thing -- but it would certainly make
me happier to find a more Rust-y way to approach this.)
Thinking about it for a little bit longer, I'm leaning a bit more
towards option 3, just because I think it would be a better idea to
associate a trait and a socket type. My gut feeling (keeping in mind my
very brief knowledge of traits, and even less in that of callbacks) is
that this would end up being nicer to implement, though maybe not as
simple.
> You might also be interested in implementing one of them. Would be a
> good starter crosvm exercise. It's probably the type of thing where you
> could be pretty confident you'd got it right if it compiled, but I could
> offer some guidance on testing it as well, if you wanted. If not,
> though, please give me your thoughts on approach anyway, and I'll
> implement whichever one we think is best myself at some point. :)
Although I would be interested in implementing one of them (#3,
specifically), time is not on my side at the moment. I've recently
started my first 40-hour-per-week job, so I'd like to adjust to that
schedule before I really start doing things again.
> I'll definitely wait longer to push crosvm patches in future, now that I
> know somebody might be looking at them!
Don't worry about it; it's because I took so long to respond. :P
Cole
P.S. I've switched to composing emails using notmuch inside Emacs. There
should be no more whitespace weirdness (though we'll find out when the next
patch comes through for sure ;^) ).
P.P.S. Thanks for going into such detail in all your responses (and
apologies for not doing the same)!
next prev parent reply other threads:[~2020-06-22 22:07 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-14 11:43 [PATCH crosvm 0/2] Fix deadlock on early VmRequest Alyssa Ross
2020-06-14 11:43 ` [PATCH crosvm 1/2] msg_socket: introduce UnixSeqpacketExt Alyssa Ross
2020-06-16 0:17 ` Cole Helbling
2020-06-16 9:32 ` Alyssa Ross
2020-06-22 22:06 ` Cole Helbling [this message]
2020-06-23 2:32 ` Alyssa Ross
2020-06-25 1:54 ` impaqt
2020-07-09 13:24 ` Alyssa Ross
2020-06-14 11:43 ` [PATCH crosvm 2/2] crosvm: fix deadlock on early VmRequest Alyssa Ross
2020-06-16 1:08 ` Cole Helbling
2020-06-16 9:39 ` Alyssa Ross
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CH2PR14MB3579DD94ED9CDFE54ADAB43BB3970@CH2PR14MB3579.namprd14.prod.outlook.com \
--to=cole.e.helbling@outlook.com \
--cc=devel@spectrum-os.org \
--cc=hi@alyssa.is \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://spectrum-os.org/git/crosvm
https://spectrum-os.org/git/doc
https://spectrum-os.org/git/mktuntap
https://spectrum-os.org/git/nixpkgs
https://spectrum-os.org/git/spectrum
https://spectrum-os.org/git/ucspi-vsock
https://spectrum-os.org/git/www
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).