* [PATCH 1/2] libsupport: Add xgetpeername @ 2024-04-08 10:25 Sergey Kolosov 2024-04-08 10:25 ` [PATCH 2/2] socket: Add new test for connect Sergey Kolosov 2024-04-09 12:31 ` [PATCH 1/2] libsupport: Add xgetpeername Arjun Shankar 0 siblings, 2 replies; 4+ messages in thread From: Sergey Kolosov @ 2024-04-08 10:25 UTC (permalink / raw) To: libc-alpha; +Cc: Sergey Kolosov The patch adds redirections for getpeername. --- support/Makefile | 1 + support/xgetpeername.c | 30 ++++++++++++++++++++++++++++++ support/xsocket.h | 1 + 3 files changed, 32 insertions(+) create mode 100644 support/xgetpeername.c diff --git a/support/Makefile b/support/Makefile index 362a51f882..aa57207bdc 100644 --- a/support/Makefile +++ b/support/Makefile @@ -131,6 +131,7 @@ libsupport-routines = \ xfreopen \ xftruncate \ xgetline \ + xgetpeername \ xgetsockname \ xlisten \ xlseek \ diff --git a/support/xgetpeername.c b/support/xgetpeername.c new file mode 100644 index 0000000000..6f448e456a --- /dev/null +++ b/support/xgetpeername.c @@ -0,0 +1,30 @@ +/* getpeername with error checking. + Copyright (C) 2024 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <https://www.gnu.org/licenses/>. */ + +#include <support/xsocket.h> + +#include <stdio.h> +#include <stdlib.h> +#include <support/check.h> + +void +xgetpeername (int fd, struct sockaddr *sa, socklen_t *plen) +{ + if (getpeername (fd, sa, plen) != 0) + FAIL_EXIT1 ("getpeername (%d): %m", fd); +} diff --git a/support/xsocket.h b/support/xsocket.h index 3e44103546..4ac0e1f5ff 100644 --- a/support/xsocket.h +++ b/support/xsocket.h @@ -26,6 +26,7 @@ int xsocket (int, int, int); void xsetsockopt (int, int, int, const void *, socklen_t); void xgetsockname (int, struct sockaddr *, socklen_t *); +void xgetpeername (int, struct sockaddr *, socklen_t *); void xconnect (int, const struct sockaddr *, socklen_t); void xbind (int, const struct sockaddr *, socklen_t); void xlisten (int, int); -- 2.44.0 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 2/2] socket: Add new test for connect 2024-04-08 10:25 [PATCH 1/2] libsupport: Add xgetpeername Sergey Kolosov @ 2024-04-08 10:25 ` Sergey Kolosov 2024-04-10 10:51 ` Arjun Shankar 2024-04-09 12:31 ` [PATCH 1/2] libsupport: Add xgetpeername Arjun Shankar 1 sibling, 1 reply; 4+ messages in thread From: Sergey Kolosov @ 2024-04-08 10:25 UTC (permalink / raw) To: libc-alpha; +Cc: Sergey Kolosov This commit adds a new test for the connect function. --- socket/Makefile | 1 + socket/tst-connect.c | 109 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+) create mode 100644 socket/tst-connect.c diff --git a/socket/Makefile b/socket/Makefile index 74ca5b8452..fc1bd0a260 100644 --- a/socket/Makefile +++ b/socket/Makefile @@ -70,6 +70,7 @@ tests := \ tst-accept4 \ tst-cmsg_cloexec \ tst-cmsghdr \ + tst-connect \ tst-sockopt \ # tests diff --git a/socket/tst-connect.c b/socket/tst-connect.c new file mode 100644 index 0000000000..44999d2953 --- /dev/null +++ b/socket/tst-connect.c @@ -0,0 +1,109 @@ +/* Test the connect function. + Copyright (C) 2024 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <https://www.gnu.org/licenses/>. */ + +#include <arpa/inet.h> +#include <errno.h> +#include <fcntl.h> +#include <signal.h> +#include <stdbool.h> +#include <support/check.h> +#include <support/xsocket.h> +#include <support/xunistd.h> +#include <sys/socket.h> +#include <stdio.h> + +static struct sockaddr_in server_address; + +int +open_socket (int domain, int type, int protocol) +{ + int fd = socket (domain, type, protocol); + if (fd < 0) + { + if (errno == EAFNOSUPPORT) + FAIL_UNSUPPORTED ("The host does not support IPv4"); + else + FAIL_EXIT1 ("socket (%d, %d, %d): %m\n", domain, type, protocol); + } + return fd; +} + +static pid_t +start_server (void) +{ + + server_address.sin_family = AF_INET; + server_address.sin_port = 0; + server_address.sin_addr.s_addr = htonl (INADDR_LOOPBACK); + + int server_sock = open_socket (AF_INET, SOCK_STREAM, IPPROTO_TCP); + + xbind (server_sock, (struct sockaddr *) &server_address, + sizeof (server_address)); + + socklen_t sa_len = sizeof (server_address); + xgetsockname (server_sock, (struct sockaddr *) &server_address, &sa_len); + printf ("Server listen on port %d\n",ntohs (server_address.sin_port)); + xlisten (server_sock, 5); + + pid_t my_pid = xfork (); + if (my_pid > 0) + { + xclose (server_sock); + return my_pid; + } + + struct sockaddr_in client_address; + socklen_t ca_len = sizeof (server_address); + int client_sock = xaccept (server_sock, + (struct sockaddr *) &client_address, &ca_len); + printf ("socket accepted %d\n",client_sock); + + _exit (0); +} + +static int +do_test (void) +{ + pid_t serv_pid; + struct sockaddr_in peer; + socklen_t peer_len; + + serv_pid = start_server (); + int client_sock = open_socket (AF_INET, SOCK_STREAM, IPPROTO_TCP); + xconnect (client_sock, (const struct sockaddr *) &server_address, + sizeof (server_address)); + int result = connect (client_sock, (const struct sockaddr *) &server_address, + sizeof (server_address)); + if (result == 0 || errno != EISCONN) + FAIL_EXIT1 ("Second connect (%d), should fail with EISCONN: %m", + client_sock); + + peer_len = sizeof (peer); + xgetpeername (client_sock, (struct sockaddr *) &peer, &peer_len); + TEST_COMPARE (peer_len, sizeof (peer)); + TEST_COMPARE (peer.sin_port, server_address.sin_port); + TEST_COMPARE_BLOB (&peer.sin_addr, sizeof (peer.sin_addr), + &server_address.sin_addr, sizeof (server_address.sin_addr)); + + int status; + xwaitpid (serv_pid, &status, 0); + TEST_COMPARE (status, 0); + return 0; +} +#include <support/test-driver.c> -- 2.44.0 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] socket: Add new test for connect 2024-04-08 10:25 ` [PATCH 2/2] socket: Add new test for connect Sergey Kolosov @ 2024-04-10 10:51 ` Arjun Shankar 0 siblings, 0 replies; 4+ messages in thread From: Arjun Shankar @ 2024-04-10 10:51 UTC (permalink / raw) To: Sergey Kolosov; +Cc: libc-alpha > This commit adds a new test for the connect function. Overall, this looks fine to me. I have a couple of suggestions. > diff --git a/socket/Makefile b/socket/Makefile > index 74ca5b8452..fc1bd0a260 100644 > --- a/socket/Makefile > +++ b/socket/Makefile > @@ -70,6 +70,7 @@ tests := \ > tst-accept4 \ > tst-cmsg_cloexec \ > tst-cmsghdr \ > + tst-connect \ > tst-sockopt \ > # tests OK. New test. > diff --git a/socket/tst-connect.c b/socket/tst-connect.c > new file mode 100644 > index 0000000000..44999d2953 > --- /dev/null > +++ b/socket/tst-connect.c > @@ -0,0 +1,109 @@ > +/* Test the connect function. > + Copyright (C) 2024 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <https://www.gnu.org/licenses/>. */ > + > +#include <arpa/inet.h> > +#include <errno.h> > +#include <fcntl.h> > +#include <signal.h> > +#include <stdbool.h> > +#include <support/check.h> > +#include <support/xsocket.h> > +#include <support/xunistd.h> > +#include <sys/socket.h> > +#include <stdio.h> > + > +static struct sockaddr_in server_address; > + > +int > +open_socket (int domain, int type, int protocol) > +{ > + int fd = socket (domain, type, protocol); > + if (fd < 0) > + { > + if (errno == EAFNOSUPPORT) > + FAIL_UNSUPPORTED ("The host does not support IPv4"); Considering we are assuming inside open_socket that we will be using IPv4, and since both calls to open_socket are with the same args, perhaps let's get rid of args for open_socket? > + else > + FAIL_EXIT1 ("socket (%d, %d, %d): %m\n", domain, type, protocol); > + } > + return fd; > +} > + > +static pid_t > +start_server (void) > +{ > + > + server_address.sin_family = AF_INET; > + server_address.sin_port = 0; > + server_address.sin_addr.s_addr = htonl (INADDR_LOOPBACK); > + > + int server_sock = open_socket (AF_INET, SOCK_STREAM, IPPROTO_TCP); > + > + xbind (server_sock, (struct sockaddr *) &server_address, > + sizeof (server_address)); > + > + socklen_t sa_len = sizeof (server_address); > + xgetsockname (server_sock, (struct sockaddr *) &server_address, &sa_len); > + printf ("Server listen on port %d\n",ntohs (server_address.sin_port)); > + xlisten (server_sock, 5); The printf doesn't seem necessary. OK otherwise. > + > + pid_t my_pid = xfork (); > + if (my_pid > 0) > + { > + xclose (server_sock); > + return my_pid; OK. Parent returns PID of child. > + } > + > + struct sockaddr_in client_address; > + socklen_t ca_len = sizeof (server_address); > + int client_sock = xaccept (server_sock, > + (struct sockaddr *) &client_address, &ca_len); > + printf ("socket accepted %d\n",client_sock); > + > + _exit (0); > +} OK. Child waits for a connection, then exits. This printf can be skipped too. > + > +static int > +do_test (void) > +{ > + pid_t serv_pid; > + struct sockaddr_in peer; > + socklen_t peer_len; > + > + serv_pid = start_server (); > + int client_sock = open_socket (AF_INET, SOCK_STREAM, IPPROTO_TCP); > + xconnect (client_sock, (const struct sockaddr *) &server_address, > + sizeof (server_address)); OK. > + int result = connect (client_sock, (const struct sockaddr *) &server_address, > + sizeof (server_address)); > + if (result == 0 || errno != EISCONN) > + FAIL_EXIT1 ("Second connect (%d), should fail with EISCONN: %m", > + client_sock); OK. Testing that a second connect fails. Took me a long moment to realize why there's a second connect call right after an xconnect. Maybe worth a comment :) > + > + peer_len = sizeof (peer); > + xgetpeername (client_sock, (struct sockaddr *) &peer, &peer_len); > + TEST_COMPARE (peer_len, sizeof (peer)); > + TEST_COMPARE (peer.sin_port, server_address.sin_port); > + TEST_COMPARE_BLOB (&peer.sin_addr, sizeof (peer.sin_addr), > + &server_address.sin_addr, sizeof (server_address.sin_addr)); OK. Checking that the peer we just connected to matches the server. > + > + int status; > + xwaitpid (serv_pid, &status, 0); > + TEST_COMPARE (status, 0); > + return 0; > +} > +#include <support/test-driver.c> OK. -- Arjun Shankar he/him/his ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] libsupport: Add xgetpeername 2024-04-08 10:25 [PATCH 1/2] libsupport: Add xgetpeername Sergey Kolosov 2024-04-08 10:25 ` [PATCH 2/2] socket: Add new test for connect Sergey Kolosov @ 2024-04-09 12:31 ` Arjun Shankar 1 sibling, 0 replies; 4+ messages in thread From: Arjun Shankar @ 2024-04-09 12:31 UTC (permalink / raw) To: Sergey Kolosov; +Cc: libc-alpha Hi Sergey, > The patch adds redirections for getpeername. This looks good to me. Reviewed-by: Arjun Shankar <arjun@redhat.com> > diff --git a/support/Makefile b/support/Makefile > index 362a51f882..aa57207bdc 100644 > --- a/support/Makefile > +++ b/support/Makefile > @@ -131,6 +131,7 @@ libsupport-routines = \ > xfreopen \ > xftruncate \ > xgetline \ > + xgetpeername \ > xgetsockname \ > xlisten \ > xlseek \ OK. New routine. > diff --git a/support/xgetpeername.c b/support/xgetpeername.c > new file mode 100644 > index 0000000000..6f448e456a > --- /dev/null > +++ b/support/xgetpeername.c > @@ -0,0 +1,30 @@ > +/* getpeername with error checking. > + Copyright (C) 2024 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <https://www.gnu.org/licenses/>. */ > + > +#include <support/xsocket.h> > + > +#include <stdio.h> > +#include <stdlib.h> > +#include <support/check.h> > + > +void > +xgetpeername (int fd, struct sockaddr *sa, socklen_t *plen) > +{ > + if (getpeername (fd, sa, plen) != 0) > + FAIL_EXIT1 ("getpeername (%d): %m", fd); > +} OK. I compared this with xgetsockname and it's in line with that. > diff --git a/support/xsocket.h b/support/xsocket.h > index 3e44103546..4ac0e1f5ff 100644 > --- a/support/xsocket.h > +++ b/support/xsocket.h > @@ -26,6 +26,7 @@ > int xsocket (int, int, int); > void xsetsockopt (int, int, int, const void *, socklen_t); > void xgetsockname (int, struct sockaddr *, socklen_t *); > +void xgetpeername (int, struct sockaddr *, socklen_t *); > void xconnect (int, const struct sockaddr *, socklen_t); > void xbind (int, const struct sockaddr *, socklen_t); > void xlisten (int, int); OK. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-04-10 10:51 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-04-08 10:25 [PATCH 1/2] libsupport: Add xgetpeername Sergey Kolosov 2024-04-08 10:25 ` [PATCH 2/2] socket: Add new test for connect Sergey Kolosov 2024-04-10 10:51 ` Arjun Shankar 2024-04-09 12:31 ` [PATCH 1/2] libsupport: Add xgetpeername Arjun Shankar
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 read-only IMAP folder(s) and NNTP newsgroup(s).