March 2021 - Spectrum Development

[PATCH ucspi-vsock] vsockserver: use fclose instead of close
by Alyssa Ross 10 Mar '21

10 Mar '21

stdio can buffer output, so if we close stdout without going through stdio, there might be buffered output that is never written. --- vsockserver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vsockserver.c b/vsockserver.c index dd9a74a..196056a 100644 --- a/vsockserver.c +++ b/vsockserver.c @@ -90,7 +90,7 @@ int main(int argc, char *argv[]) if (notify) { printf("%" PRIu32 "\n", lport); - close(STDOUT_FILENO); + fclose(stdout); } setenvf("VSOCKLOCALCID", 1, "%" PRIu32, lcid); -- 2.30.0

2 2

[PATCH ucspi-vsock] Makefile: link object files instead of source files
by Alyssa Ross 10 Mar '21

10 Mar '21

Previously this would (I assume) needlessly recompile vsockserver.c and vsockclient.c when Make has already automatically compiled those. GNU Make has an automatic variable, $+, that we could use here, but currently the Makefile is portable, and it would be a shame to require GNU Make just for that. --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index b931c89..4f11b64 100644 --- a/Makefile +++ b/Makefile @@ -19,9 +19,9 @@ install: vsockclient vsockserver .PHONY: install vsockclient: vsockclient.o env.o log.o util.o vsock.o - $(CC) $(LDFLAGS) -o vsockclient vsockclient.c env.o log.o util.o vsock.o $(LDLIBS) + $(CC) $(LDFLAGS) -o vsockclient vsockclient.o env.o log.o util.o vsock.o $(LDLIBS) vsockserver: vsockserver.o env.o log.o util.o vsock.o - $(CC) $(LDFLAGS) -o vsockserver vsockserver.c env.o log.o util.o vsock.o $(LDLIBS) + $(CC) $(LDFLAGS) -o vsockserver vsockserver.o env.o log.o util.o vsock.o $(LDLIBS) vsockclient.o: env.h log.h util.h vsock.h vsockserver.o: env.h log.h util.h vsock.h -- 2.30.0

2 1

[PATCH ucspi-vsock] vsock_accept: return fd instead of 0 on success
by Alyssa Ross 10 Mar '21

10 Mar '21

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… 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

2 1

[PATCH ucspi-vsock] vsock_open: return fd instead of 0 on success
by Alyssa Ross 10 Mar '21

10 Mar '21

Previously, this returned the return value of connect(2), which is 0 on success. So stdin would be assigned to descriptors 6 and 7 in vsockclient instead of the socket. Oops. --- vsock.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/vsock.c b/vsock.c index 3bcd8b3..d9ff3b6 100644 --- a/vsock.c +++ b/vsock.c @@ -55,7 +55,10 @@ int vsock_open(uint32_t cid, uint32_t port) if (fd == -1) return -1; - return vsock_connect(fd, cid, port); + if (vsock_connect(fd, cid, port) == -1) + return -1; + + return fd; } int vsock_get_port(int fd, uint32_t *port) -- 2.30.0

2 1