| Message ID | 20210110001852.35653-7-dsahern@kernel.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | selftests: Updates to allow single instance of nettest for client and server | expand |
On Sat, 9 Jan 2021 17:18:47 -0700 David Ahern wrote: > From: David Ahern <dsahern@gmail.com> > > When a single instance of nettest is doing both client and > server modes, stdout and stderr messages can get interlaced > and become unreadable. Allocate a new set of buffers for the > child process handling server mode. > > Signed-off-by: David Ahern <dsahern@gmail.com> > --- > tools/testing/selftests/net/nettest.c | 20 +++++++++++++++++++- > 1 file changed, 19 insertions(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/net/nettest.c b/tools/testing/selftests/net/nettest.c > index 0e4196027d63..13c74774e357 100644 > --- a/tools/testing/selftests/net/nettest.c > +++ b/tools/testing/selftests/net/nettest.c > @@ -1705,9 +1705,27 @@ static char *random_msg(int len) > > static int ipc_child(int fd, struct sock_args *args) > { > + char *outbuf, *errbuf; > + int rc; > + > + outbuf = malloc(4096); > + errbuf = malloc(4096); > + if (!outbuf || !errbuf) { > + fprintf(stderr, "server: Failed to allocate buffers for stdout and stderr\n"); > + return 1; that's a memleak, rc = 1, goto free; ? Also there's a few uses of fprintf(stderr, .. instead of log_error() is there a reason for it? I don't think this is a big deal, I'll apply unless you object in time. > + } > + > + setbuffer(stdout, outbuf, 4096); > + setbuffer(stderr, errbuf, 4096); > + > server_mode = 1; /* to tell log_msg in case we are in both_mode */ > > - return do_server(args, fd); > + rc = do_server(args, fd); > + > + free(outbuf); > + free(errbuf); > + > + return rc; > } > > static int ipc_parent(int cpid, int fd, struct sock_args *args)
On 1/11/21 5:15 PM, Jakub Kicinski wrote: >> diff --git a/tools/testing/selftests/net/nettest.c b/tools/testing/selftests/net/nettest.c >> index 0e4196027d63..13c74774e357 100644 >> --- a/tools/testing/selftests/net/nettest.c >> +++ b/tools/testing/selftests/net/nettest.c >> @@ -1705,9 +1705,27 @@ static char *random_msg(int len) >> >> static int ipc_child(int fd, struct sock_args *args) >> { >> + char *outbuf, *errbuf; >> + int rc; >> + >> + outbuf = malloc(4096); >> + errbuf = malloc(4096); >> + if (!outbuf || !errbuf) { >> + fprintf(stderr, "server: Failed to allocate buffers for stdout and stderr\n"); >> + return 1; > > that's a memleak, rc = 1, goto free; ? > > Also there's a few uses of fprintf(stderr, .. instead of log_error() > is there a reason for it? > > I don't think this is a big deal, I'll apply unless you object in time. > good catch. I found a bug in patch 5 as well. I'll fix and re-send.
diff --git a/tools/testing/selftests/net/nettest.c b/tools/testing/selftests/net/nettest.c index 0e4196027d63..13c74774e357 100644 --- a/tools/testing/selftests/net/nettest.c +++ b/tools/testing/selftests/net/nettest.c @@ -1705,9 +1705,27 @@ static char *random_msg(int len) static int ipc_child(int fd, struct sock_args *args) { + char *outbuf, *errbuf; + int rc; + + outbuf = malloc(4096); + errbuf = malloc(4096); + if (!outbuf || !errbuf) { + fprintf(stderr, "server: Failed to allocate buffers for stdout and stderr\n"); + return 1; + } + + setbuffer(stdout, outbuf, 4096); + setbuffer(stderr, errbuf, 4096); + server_mode = 1; /* to tell log_msg in case we are in both_mode */ - return do_server(args, fd); + rc = do_server(args, fd); + + free(outbuf); + free(errbuf); + + return rc; } static int ipc_parent(int cpid, int fd, struct sock_args *args)