| Message ID | 20240912171251.937743-6-sdf@fomichev.me (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | selftests: ncdevmem: Add ncdevmem to ksft | expand |
On Thu, Sep 12, 2024 at 10:13 AM Stanislav Fomichev <sdf@fomichev.me> wrote: > > There is a bunch of places where error() calls look out of place. > Use the same error(1, errno, ...) pattern everywhere. > > Cc: Mina Almasry <almasrymina@google.com> > Signed-off-by: Stanislav Fomichev <sdf@fomichev.me> > --- > tools/testing/selftests/net/ncdevmem.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/tools/testing/selftests/net/ncdevmem.c b/tools/testing/selftests/net/ncdevmem.c > index a20f40adfde8..c0da2b2e077f 100644 > --- a/tools/testing/selftests/net/ncdevmem.c > +++ b/tools/testing/selftests/net/ncdevmem.c > @@ -332,32 +332,32 @@ int do_server(struct memory_buffer *mem) > > ret = inet_pton(server_sin.sin_family, server_ip, &server_sin.sin_addr); > if (socket < 0) > - error(79, 0, "%s: [FAIL, create socket]\n", TEST_PREFIX); > + error(1, 0, "%s: [FAIL, create socket]\n", TEST_PREFIX); > > socket_fd = socket(server_sin.sin_family, SOCK_STREAM, 0); > if (socket < 0) > - error(errno, errno, "%s: [FAIL, create socket]\n", TEST_PREFIX); > + error(1, errno, "%s: [FAIL, create socket]\n", TEST_PREFIX); > To be honest this was a bit intentional. For example here, I want to see what errno socket() failed with; it's a clue to why it failed. I guess you're not actually removing that, right? You're making the return code of ncdevmem 1, but it will still print the errno of the subfailure? That sounds fine. Isn't it a bit more standard for the sub errno to be the return of the parent process. But not opposed. If you think this is better we can go for it. -- Thanks, Mina
On 09/12, Mina Almasry wrote: > On Thu, Sep 12, 2024 at 10:13 AM Stanislav Fomichev <sdf@fomichev.me> wrote: > > > > There is a bunch of places where error() calls look out of place. > > Use the same error(1, errno, ...) pattern everywhere. > > > > Cc: Mina Almasry <almasrymina@google.com> > > Signed-off-by: Stanislav Fomichev <sdf@fomichev.me> > > --- > > tools/testing/selftests/net/ncdevmem.c | 12 ++++++------ > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/tools/testing/selftests/net/ncdevmem.c b/tools/testing/selftests/net/ncdevmem.c > > index a20f40adfde8..c0da2b2e077f 100644 > > --- a/tools/testing/selftests/net/ncdevmem.c > > +++ b/tools/testing/selftests/net/ncdevmem.c > > @@ -332,32 +332,32 @@ int do_server(struct memory_buffer *mem) > > > > ret = inet_pton(server_sin.sin_family, server_ip, &server_sin.sin_addr); > > if (socket < 0) > > - error(79, 0, "%s: [FAIL, create socket]\n", TEST_PREFIX); > > + error(1, 0, "%s: [FAIL, create socket]\n", TEST_PREFIX); > > > > socket_fd = socket(server_sin.sin_family, SOCK_STREAM, 0); > > if (socket < 0) > > - error(errno, errno, "%s: [FAIL, create socket]\n", TEST_PREFIX); > > + error(1, errno, "%s: [FAIL, create socket]\n", TEST_PREFIX); > > > > To be honest this was a bit intentional. For example here, I want to > see what errno socket() failed with; it's a clue to why it failed. > > I guess you're not actually removing that, right? You're making the > return code of ncdevmem 1, but it will still print the errno of the > subfailure? That sounds fine. > > Isn't it a bit more standard for the sub errno to be the return of the > parent process. But not opposed. If you think this is better we can go > for it. Yes, this will still print the correct errno.
diff --git a/tools/testing/selftests/net/ncdevmem.c b/tools/testing/selftests/net/ncdevmem.c index a20f40adfde8..c0da2b2e077f 100644 --- a/tools/testing/selftests/net/ncdevmem.c +++ b/tools/testing/selftests/net/ncdevmem.c @@ -332,32 +332,32 @@ int do_server(struct memory_buffer *mem) ret = inet_pton(server_sin.sin_family, server_ip, &server_sin.sin_addr); if (socket < 0) - error(79, 0, "%s: [FAIL, create socket]\n", TEST_PREFIX); + error(1, 0, "%s: [FAIL, create socket]\n", TEST_PREFIX); socket_fd = socket(server_sin.sin_family, SOCK_STREAM, 0); if (socket < 0) - error(errno, errno, "%s: [FAIL, create socket]\n", TEST_PREFIX); + error(1, errno, "%s: [FAIL, create socket]\n", TEST_PREFIX); ret = setsockopt(socket_fd, SOL_SOCKET, SO_REUSEPORT, &opt, sizeof(opt)); if (ret) - error(errno, errno, "%s: [FAIL, set sock opt]\n", TEST_PREFIX); + error(1, errno, "%s: [FAIL, set sock opt]\n", TEST_PREFIX); ret = setsockopt(socket_fd, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt)); if (ret) - error(errno, errno, "%s: [FAIL, set sock opt]\n", TEST_PREFIX); + error(1, errno, "%s: [FAIL, set sock opt]\n", TEST_PREFIX); fprintf(stderr, "binding to address %s:%d\n", server_ip, ntohs(server_sin.sin_port)); ret = bind(socket_fd, &server_sin, sizeof(server_sin)); if (ret) - error(errno, errno, "%s: [FAIL, bind]\n", TEST_PREFIX); + error(1, errno, "%s: [FAIL, bind]\n", TEST_PREFIX); ret = listen(socket_fd, 1); if (ret) - error(errno, errno, "%s: [FAIL, listen]\n", TEST_PREFIX); + error(1, errno, "%s: [FAIL, listen]\n", TEST_PREFIX); client_addr_len = sizeof(client_addr);
There is a bunch of places where error() calls look out of place. Use the same error(1, errno, ...) pattern everywhere. Cc: Mina Almasry <almasrymina@google.com> Signed-off-by: Stanislav Fomichev <sdf@fomichev.me> --- tools/testing/selftests/net/ncdevmem.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)