* [PATCH ucspi-vsock] vsock_accept: return fd instead of 0 on success
@ 2021-03-10 20:45 Alyssa Ross
2021-03-10 20:48 ` Cole Helbling
0 siblings, 1 reply; 2+ messages in thread
From: Alyssa Ross @ 2021-03-10 20:45 UTC (permalink / raw)
To: devel; +Cc: Cole Helbling
This would result in the spawned process being hooked up to stdin,
instead of the vsock. Then stdin would be closed, so subsequent
processes would be connected to nothing. Oops.
---
I am... a bit embarassed that I didn't notice this after already
fixing the exact same problem in vsock_connect[1].
Maybe I can write an integration test that runs vsockserver and
vsockclient and checks they actually do the right thing...
[1]: https://spectrum-os.org/lists/archives/spectrum-devel/20210309171816.8589-1-hi@alyssa.is/raw
vsock.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/vsock.c b/vsock.c
index d9ff3b6..e6a173c 100644
--- a/vsock.c
+++ b/vsock.c
@@ -32,14 +32,15 @@ int vsock_accept(int sockfd, uint32_t *cid, uint32_t *port)
{
struct sockaddr_vm addr = { 0 };
socklen_t addr_size = sizeof addr;
+ int fd;
- if (accept(sockfd, (struct sockaddr *)&addr, &addr_size) == -1)
+ if ((fd = accept(sockfd, (struct sockaddr *)&addr, &addr_size)) == -1)
return -1;
*cid = addr.svm_cid;
*port = addr.svm_port;
- return 0;
+ return fd;
}
int vsock_connect(int fd, uint32_t cid, uint32_t port)
--
2.30.0
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH ucspi-vsock] vsock_accept: return fd instead of 0 on success
2021-03-10 20:45 [PATCH ucspi-vsock] vsock_accept: return fd instead of 0 on success Alyssa Ross
@ 2021-03-10 20:48 ` Cole Helbling
0 siblings, 0 replies; 2+ messages in thread
From: Cole Helbling @ 2021-03-10 20:48 UTC (permalink / raw)
To: Alyssa Ross, devel; +Cc: Cole Helbling
On Wed Mar 10, 2021 at 12:45 PM PST, Alyssa Ross wrote:
> This would result in the spawned process being hooked up to stdin,
> instead of the vsock. Then stdin would be closed, so subsequent
> processes would be connected to nothing. Oops.
>
> ---
> I am... a bit embarassed that I didn't notice this after already
> fixing the exact same problem in vsock_connect[1].
Technically, you _did_ notice this after fixing the the problem in
vsock_connect -- just not immediately / at the same time ;)
> Maybe I can write an integration test that runs vsockserver and
> vsockclient and checks they actually do the right thing...
>
> [1]:
> https://spectrum-os.org/lists/archives/spectrum-devel/20210309171816.8589-1-hi@alyssa.is/raw
>
> vsock.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/vsock.c b/vsock.c
> index d9ff3b6..e6a173c 100644
> --- a/vsock.c
> +++ b/vsock.c
> @@ -32,14 +32,15 @@ int vsock_accept(int sockfd, uint32_t *cid, uint32_t
> *port)
> {
> struct sockaddr_vm addr = { 0 };
> socklen_t addr_size = sizeof addr;
> + int fd;
>
> - if (accept(sockfd, (struct sockaddr *)&addr, &addr_size) == -1)
> + if ((fd = accept(sockfd, (struct sockaddr *)&addr, &addr_size)) == -1)
> return -1;
>
> *cid = addr.svm_cid;
> *port = addr.svm_port;
>
> - return 0;
> + return fd;
> }
>
> int vsock_connect(int fd, uint32_t cid, uint32_t port)
> --
> 2.30.0
Reviewed-by: Cole Helbling <cole.e.helbling@outlook.com>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-03-10 20:50 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-10 20:45 [PATCH ucspi-vsock] vsock_accept: return fd instead of 0 on success Alyssa Ross
2021-03-10 20:48 ` Cole Helbling
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).