From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 4065E3858CDB for ; Wed, 10 Apr 2024 10:51:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4065E3858CDB Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 4065E3858CDB Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1712746289; cv=none; b=btf2+TE90axqiCKAxIV2njOspn5yDtFFLTa+fH9XUnnFY78dOYH8W/in88xZjazpsrKzwVTCofEanJS2YKCLs7Bvr9+pkroxg8y+ZSjKf6pGMO6KvDxL8XYI0REbs0whxR/W+2BhDOgzpRkvIdM9LIrlmOCYKxj0V5J7A/1/4Mk= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1712746289; c=relaxed/simple; bh=ai9MUNBidm+PpG+aBj+4JHiWpppsU/cEJIS9nOL5C/8=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=tIs9VnIi4QRupo494aAsBEq42T8xhucSkhu6nhmmORebT0mDFKJ21ZqeaLIrCnWkolAqvwqNOZRJOGho565u9eajlB7Z1js80iENhlhvxSnzSh67e1CyvlFvL+BrTuqM2HTChD1pa0owbuIp3hdxS9BvLjhXTEdZACVwmbwrtQ4= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1712746287; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=VGmBWTRnjl3KmGk+DfxEZV2Ok0CxBSJSBljLxPMmGA8=; b=i80p++hXFOh750euPqabk7qxMhY4/XDWSJlMCYl21ety+sxtlCq0/roWQxzNkNWBUgpQY4 LzG/g7tAzJxGviPFlOWdw/rLhjx2A9xqt+0Lh0xdOnAwl3KNaEwDNatkN+ZsXh9nufuID/ Nwrvc+YzIdpa2BSy+4d8y9tPwNI2UjQ= Received: from mail-oo1-f72.google.com (mail-oo1-f72.google.com [209.85.161.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-683-qgZV86K_P3WlPNcbaQmJpw-1; Wed, 10 Apr 2024 06:51:26 -0400 X-MC-Unique: qgZV86K_P3WlPNcbaQmJpw-1 Received: by mail-oo1-f72.google.com with SMTP id 006d021491bc7-5a7ba13c414so5274040eaf.0 for ; Wed, 10 Apr 2024 03:51:26 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712746285; x=1713351085; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=VGmBWTRnjl3KmGk+DfxEZV2Ok0CxBSJSBljLxPMmGA8=; b=WoiNauD1ZxOzItJebVy7oP2V8BxusonRQIn1tYOOUkT0LjFqXmRmSrixUVelmwMYgz zj8IdNjz/69SuP5UHmTwVZQk0sqZQ5E0Q62K8zLTr9g9Hq9jwId5uujz9GTXCUi0JZVn ThzgmZBW9mJp54poIDi2YrFGtKGNnqpiI7muttxYEcCgDjBPiLWIUvEu9X3ahjtBjoGh jajxQqeHweSA6fGcDOUe44ouHNp3pC+P3JyZWn+U5J8YJDAebpvUOxQ1Y11pN35Piu49 Pp6q6uQQ5UDSQHoDFHJD3dIUE6+vfZpPorNbT6TpnfcRmO8FTZghF86Q+Cp+dQawtwC3 zw0w== X-Gm-Message-State: AOJu0Yx5WH4y2gaiPlpzRGmYfJYXZW5L0lci3kts8tNPTccEgRZRm/FD uuC8/ovvt8CbfaIo7pJuy18wDUcg5/WyLpuOaO/5mJFiRzmx7b0iwKiU8jbCSqY/BG1mRZ0C9v3 qLHRE2Bv7cGoUMaaCIz5Bdg0Z32nY1fWYqkGB4ziCMXwHBfmwLEDC9d/cHTsgNasOyNyr0zehuB jS5c7l9sicKs+Jfnk/YfbDdB/ev9yZVsoiAPZNK0gU X-Received: by 2002:a05:6820:1b10:b0:5aa:4881:2ab2 with SMTP id bv16-20020a0568201b1000b005aa48812ab2mr3698473oob.4.1712746285758; Wed, 10 Apr 2024 03:51:25 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEDMWDkOQPZZNMO+nLFA5qWMeZ9ig7TIq2g4Q2nywUar1pKQ+UTeUkLSuu5WOSYqb8AMmpcYrqEKA6qyBmpKQ0= X-Received: by 2002:a05:6820:1b10:b0:5aa:4881:2ab2 with SMTP id bv16-20020a0568201b1000b005aa48812ab2mr3698461oob.4.1712746285534; Wed, 10 Apr 2024 03:51:25 -0700 (PDT) MIME-Version: 1.0 References: <20240408102543.18143-1-skolosov@redhat.com> <20240408102543.18143-2-skolosov@redhat.com> In-Reply-To: <20240408102543.18143-2-skolosov@redhat.com> From: Arjun Shankar Date: Wed, 10 Apr 2024 12:51:14 +0200 Message-ID: Subject: Re: [PATCH 2/2] socket: Add new test for connect To: Sergey Kolosov Cc: libc-alpha@sourceware.org X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-13.3 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: > 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 > + . */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +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 OK. -- Arjun Shankar he/him/his