gem5-dev@gem5.org

The gem5 Developer List

View all threads

[M] Change in gem5/gem5[develop]: base,cpu,dev: Simplify ListenSocket::listen().

GB
Gabe Black (Gerrit)
Wed, Mar 22, 2023 9:19 AM

Gabe Black has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/69159?usp=email )

Change subject: base,cpu,dev: Simplify ListenSocket::listen().
......................................................................

base,cpu,dev: Simplify ListenSocket::listen().

Remove the "reuse" parameter which default to true and was always
also explicitly set to true. Tidy up the code itself slightly, mostly
by using "panic_if" to remove some nesting.

Change-Id: Ie23971aabf2fe4252d27f1887468360722a72379
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/69159
Maintainer: Gabe Black gabeblack@google.com
Reviewed-by: Yu-hsin Wang yuhsingw@google.com
Tested-by: kokoro noreply+kokoro@google.com

M src/base/remote_gdb.cc
M src/base/socket.cc
M src/base/socket.hh
M src/base/socket.test.cc
M src/base/vnc/vncserver.cc
M src/cpu/nativetrace.cc
M src/dev/net/ethertap.cc
M src/dev/serial/terminal.cc
8 files changed, 24 insertions(+), 38 deletions(-)

Approvals:
Yu-hsin Wang: Looks good to me, approved
kokoro: Regressions pass
Gabe Black: Looks good to me, approved

diff --git a/src/base/remote_gdb.cc b/src/base/remote_gdb.cc
index b709ac3..1a2fef4 100644
--- a/src/base/remote_gdb.cc
+++ b/src/base/remote_gdb.cc
@@ -417,7 +417,7 @@
return;
}

  • while (!listener.listen(_port, true)) {
  • while (!listener.listen(_port)) {
    DPRINTF(GDBMisc, "Can't bind port %d\n", _port);
    _port++;
    }
    diff --git a/src/base/socket.cc b/src/base/socket.cc
    index 0a62a88..280f92b 100644
    --- a/src/base/socket.cc
    +++ b/src/base/socket.cc
    @@ -185,24 +185,20 @@

// Create a socket and configure it for listening
bool
-ListenSocket::listen(int port, bool reuse)
+ListenSocket::listen(int port)
{

  • if (listening)
  •    panic("Socket already listening!");
    
  • panic_if(listening, "Socket already listening!");

    // only create socket if not already created by a previous call
    if (fd == -1) {
    fd = socketCloexec(PF_INET, SOCK_STREAM, 0);

  •    if (fd < 0)
    
  •        panic("Can't create socket:%s !", strerror(errno));
    
  •    panic_if(fd < 0, "Can't create socket:%s !", strerror(errno));
    }
    
  • if (reuse) {
  •    int i = 1;
    
  •    if (::setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (char *)&i,
    
  •                     sizeof(i)) < 0)
    
  •        panic("ListenSocket(listen): setsockopt() SO_REUSEADDR  
    

failed!");

  • }
  • int i = 1;
  • int ret = ::setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &i, sizeof(i));
  • panic_if(ret < 0,
  •        "ListenSocket(listen): setsockopt() SO_REUSEADDR failed!");
    
    struct sockaddr_in sockaddr;
    sockaddr.sin_family = PF_INET;
    

@@ -211,16 +207,16 @@
sockaddr.sin_port = htons(port);
// finally clear sin_zero
std::memset(&sockaddr.sin_zero, 0, sizeof(sockaddr.sin_zero));

  • int ret = ::bind(fd, (struct sockaddr *)&sockaddr, sizeof (sockaddr));
  • ret = ::bind(fd, (struct sockaddr *)&sockaddr, sizeof (sockaddr));
    if (ret != 0) {
  •    if (ret == -1 && errno != EADDRINUSE)
    
  •        panic("ListenSocket(listen): bind() failed!");
    
  •    panic_if(ret == -1 && errno != EADDRINUSE,
    
  •            "ListenSocket(listen): bind() failed!");
        return false;
    }
    
    if (::listen(fd, 1) == -1) {
    
  •    if (errno != EADDRINUSE)
    
  •        panic("ListenSocket(listen): listen() failed!");
    
  •    panic_if(errno != EADDRINUSE,
    
  •            "ListenSocket(listen): listen() failed!");
        // User may decide to retry with a different port later; however,  
    

the
// socket is already bound to a port and the next bind will surely
// fail. We'll close the socket and reset fd to -1 so our user can
diff --git a/src/base/socket.hh b/src/base/socket.hh
index aa451b6..d2393e9 100644
--- a/src/base/socket.hh
+++ b/src/base/socket.hh
@@ -106,7 +106,7 @@

  virtual int accept();
  • virtual bool listen(int port, bool reuse = true);
  • virtual bool listen(int port);

    int getfd() const { return fd; }
    bool islistening() const { return listening; }
    diff --git a/src/base/socket.test.cc b/src/base/socket.test.cc
    index 1ab1f21..cb24c49 100644
    --- a/src/base/socket.test.cc
    +++ b/src/base/socket.test.cc
    @@ -162,19 +162,6 @@
    EXPECT_FALSE(listen_socket.allDisabled());
    }

-TEST(SocketTest, ListenToPortReuseFalse)
-{

  • MockListenSocket listen_socket;
  • /*
  • * The ListenSocket object should have the same state regardless as to
    
  • * whether reuse is true or false (it is true by default).
    
  • */
    
  • EXPECT_TRUE(listen_socket.listen(TestPort1, false));
  • EXPECT_NE(-1, listen_socket.getfd());
  • EXPECT_TRUE(listen_socket.islistening());
  • EXPECT_FALSE(listen_socket.allDisabled());
    -}
  • TEST(SocketTest, RelistenWithSameInstanceSamePort)
    {
    MockListenSocket listen_socket;
    @@ -185,7 +172,9 @@
    */
    gtestLogOutput.str("");
    EXPECT_ANY_THROW(listen_socket.listen(TestPort1));
  • std::string expected = "panic: Socket already listening!\n";
  • std::string expected =
  •    "panic: panic condition listening occurred: "
    
  •    "Socket already listening!\n";
    std::string actual = gtestLogOutput.str();
    EXPECT_EQ(expected, actual);
    
    }
    @@ -201,7 +190,9 @@
    gtestLogOutput.str("");
    EXPECT_ANY_THROW(listen_socket.listen(TestPort2));
  • std::string expected = "panic: Socket already listening!\n";
  • std::string expected =
  •    "panic: panic condition listening occurred: "
    
  •    "Socket already listening!\n";
    std::string actual = gtestLogOutput.str();
    EXPECT_EQ(expected, actual);
    
    }
    diff --git a/src/base/vnc/vncserver.cc b/src/base/vnc/vncserver.cc
    index 5792c44..39a1338 100644
    --- a/src/base/vnc/vncserver.cc
    +++ b/src/base/vnc/vncserver.cc
    @@ -164,7 +164,7 @@
    return;
    }
  • while (!listener.listen(port, true)) {
  • while (!listener.listen(port)) {
    DPRINTF(VNC,
    "can't bind address vnc server port %d in use PID %d\n",
    port, getpid());
    diff --git a/src/cpu/nativetrace.cc b/src/cpu/nativetrace.cc
    index 5b7d0b9..714787f 100644
    --- a/src/cpu/nativetrace.cc
    +++ b/src/cpu/nativetrace.cc
    @@ -45,8 +45,7 @@
    fatal("All listeners are disabled!");

    int port = 8000;

  • while (!native_listener.listen(port, true))
  • {
  • while (!native_listener.listen(port)) {
    DPRINTF(GDBMisc, "Can't bind port %d\n", port);
    port++;
    }
    diff --git a/src/dev/net/ethertap.cc b/src/dev/net/ethertap.cc
    index b28f255..0769ad1 100644
    --- a/src/dev/net/ethertap.cc
    +++ b/src/dev/net/ethertap.cc
    @@ -259,7 +259,7 @@
    void
    TapListener::listen()
    {
  • while (!listener.listen(port, true)) {
  • while (!listener.listen(port)) {
    DPRINTF(Ethernet, "TapListener(listen): Can't bind port %d\n",
    port);
    port++;
    }
    diff --git a/src/dev/serial/terminal.cc b/src/dev/serial/terminal.cc
    index fada99c..9564876 100644
    --- a/src/dev/serial/terminal.cc
    +++ b/src/dev/serial/terminal.cc
    @@ -175,7 +175,7 @@
    return;
    }
  • while (!listener.listen(port, true)) {
  • while (!listener.listen(port)) {
    DPRINTF(Terminal,
    ": can't bind address terminal port %d inuse PID %d\n",
    port, getpid());

--
To view, visit
https://gem5-review.googlesource.com/c/public/gem5/+/69159?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: Ie23971aabf2fe4252d27f1887468360722a72379
Gerrit-Change-Number: 69159
Gerrit-PatchSet: 3
Gerrit-Owner: Gabe Black gabe.black@gmail.com
Gerrit-Reviewer: Bobby Bruce bbruce@ucdavis.edu
Gerrit-Reviewer: Daniel Carvalho odanrc@yahoo.com.br
Gerrit-Reviewer: Gabe Black gabe.black@gmail.com
Gerrit-Reviewer: Gabe Black gabeblack@google.com
Gerrit-Reviewer: Jason Lowe-Power jason@lowepower.com
Gerrit-Reviewer: Yu-hsin Wang yuhsingw@google.com
Gerrit-Reviewer: kokoro noreply+kokoro@google.com
Gerrit-MessageType: merged

Gabe Black has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/69159?usp=email ) Change subject: base,cpu,dev: Simplify ListenSocket::listen(). ...................................................................... base,cpu,dev: Simplify ListenSocket::listen(). Remove the "reuse" parameter which default to true and was always also explicitly set to true. Tidy up the code itself slightly, mostly by using "panic_if" to remove some nesting. Change-Id: Ie23971aabf2fe4252d27f1887468360722a72379 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/69159 Maintainer: Gabe Black <gabeblack@google.com> Reviewed-by: Yu-hsin Wang <yuhsingw@google.com> Tested-by: kokoro <noreply+kokoro@google.com> --- M src/base/remote_gdb.cc M src/base/socket.cc M src/base/socket.hh M src/base/socket.test.cc M src/base/vnc/vncserver.cc M src/cpu/nativetrace.cc M src/dev/net/ethertap.cc M src/dev/serial/terminal.cc 8 files changed, 24 insertions(+), 38 deletions(-) Approvals: Yu-hsin Wang: Looks good to me, approved kokoro: Regressions pass Gabe Black: Looks good to me, approved diff --git a/src/base/remote_gdb.cc b/src/base/remote_gdb.cc index b709ac3..1a2fef4 100644 --- a/src/base/remote_gdb.cc +++ b/src/base/remote_gdb.cc @@ -417,7 +417,7 @@ return; } - while (!listener.listen(_port, true)) { + while (!listener.listen(_port)) { DPRINTF(GDBMisc, "Can't bind port %d\n", _port); _port++; } diff --git a/src/base/socket.cc b/src/base/socket.cc index 0a62a88..280f92b 100644 --- a/src/base/socket.cc +++ b/src/base/socket.cc @@ -185,24 +185,20 @@ // Create a socket and configure it for listening bool -ListenSocket::listen(int port, bool reuse) +ListenSocket::listen(int port) { - if (listening) - panic("Socket already listening!"); + panic_if(listening, "Socket already listening!"); // only create socket if not already created by a previous call if (fd == -1) { fd = socketCloexec(PF_INET, SOCK_STREAM, 0); - if (fd < 0) - panic("Can't create socket:%s !", strerror(errno)); + panic_if(fd < 0, "Can't create socket:%s !", strerror(errno)); } - if (reuse) { - int i = 1; - if (::setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (char *)&i, - sizeof(i)) < 0) - panic("ListenSocket(listen): setsockopt() SO_REUSEADDR failed!"); - } + int i = 1; + int ret = ::setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &i, sizeof(i)); + panic_if(ret < 0, + "ListenSocket(listen): setsockopt() SO_REUSEADDR failed!"); struct sockaddr_in sockaddr; sockaddr.sin_family = PF_INET; @@ -211,16 +207,16 @@ sockaddr.sin_port = htons(port); // finally clear sin_zero std::memset(&sockaddr.sin_zero, 0, sizeof(sockaddr.sin_zero)); - int ret = ::bind(fd, (struct sockaddr *)&sockaddr, sizeof (sockaddr)); + ret = ::bind(fd, (struct sockaddr *)&sockaddr, sizeof (sockaddr)); if (ret != 0) { - if (ret == -1 && errno != EADDRINUSE) - panic("ListenSocket(listen): bind() failed!"); + panic_if(ret == -1 && errno != EADDRINUSE, + "ListenSocket(listen): bind() failed!"); return false; } if (::listen(fd, 1) == -1) { - if (errno != EADDRINUSE) - panic("ListenSocket(listen): listen() failed!"); + panic_if(errno != EADDRINUSE, + "ListenSocket(listen): listen() failed!"); // User may decide to retry with a different port later; however, the // socket is already bound to a port and the next bind will surely // fail. We'll close the socket and reset fd to -1 so our user can diff --git a/src/base/socket.hh b/src/base/socket.hh index aa451b6..d2393e9 100644 --- a/src/base/socket.hh +++ b/src/base/socket.hh @@ -106,7 +106,7 @@ virtual int accept(); - virtual bool listen(int port, bool reuse = true); + virtual bool listen(int port); int getfd() const { return fd; } bool islistening() const { return listening; } diff --git a/src/base/socket.test.cc b/src/base/socket.test.cc index 1ab1f21..cb24c49 100644 --- a/src/base/socket.test.cc +++ b/src/base/socket.test.cc @@ -162,19 +162,6 @@ EXPECT_FALSE(listen_socket.allDisabled()); } -TEST(SocketTest, ListenToPortReuseFalse) -{ - MockListenSocket listen_socket; - /* - * The ListenSocket object should have the same state regardless as to - * whether reuse is true or false (it is true by default). - */ - EXPECT_TRUE(listen_socket.listen(TestPort1, false)); - EXPECT_NE(-1, listen_socket.getfd()); - EXPECT_TRUE(listen_socket.islistening()); - EXPECT_FALSE(listen_socket.allDisabled()); -} - TEST(SocketTest, RelistenWithSameInstanceSamePort) { MockListenSocket listen_socket; @@ -185,7 +172,9 @@ */ gtestLogOutput.str(""); EXPECT_ANY_THROW(listen_socket.listen(TestPort1)); - std::string expected = "panic: Socket already listening!\n"; + std::string expected = + "panic: panic condition listening occurred: " + "Socket already listening!\n"; std::string actual = gtestLogOutput.str(); EXPECT_EQ(expected, actual); } @@ -201,7 +190,9 @@ gtestLogOutput.str(""); EXPECT_ANY_THROW(listen_socket.listen(TestPort2)); - std::string expected = "panic: Socket already listening!\n"; + std::string expected = + "panic: panic condition listening occurred: " + "Socket already listening!\n"; std::string actual = gtestLogOutput.str(); EXPECT_EQ(expected, actual); } diff --git a/src/base/vnc/vncserver.cc b/src/base/vnc/vncserver.cc index 5792c44..39a1338 100644 --- a/src/base/vnc/vncserver.cc +++ b/src/base/vnc/vncserver.cc @@ -164,7 +164,7 @@ return; } - while (!listener.listen(port, true)) { + while (!listener.listen(port)) { DPRINTF(VNC, "can't bind address vnc server port %d in use PID %d\n", port, getpid()); diff --git a/src/cpu/nativetrace.cc b/src/cpu/nativetrace.cc index 5b7d0b9..714787f 100644 --- a/src/cpu/nativetrace.cc +++ b/src/cpu/nativetrace.cc @@ -45,8 +45,7 @@ fatal("All listeners are disabled!"); int port = 8000; - while (!native_listener.listen(port, true)) - { + while (!native_listener.listen(port)) { DPRINTF(GDBMisc, "Can't bind port %d\n", port); port++; } diff --git a/src/dev/net/ethertap.cc b/src/dev/net/ethertap.cc index b28f255..0769ad1 100644 --- a/src/dev/net/ethertap.cc +++ b/src/dev/net/ethertap.cc @@ -259,7 +259,7 @@ void TapListener::listen() { - while (!listener.listen(port, true)) { + while (!listener.listen(port)) { DPRINTF(Ethernet, "TapListener(listen): Can't bind port %d\n", port); port++; } diff --git a/src/dev/serial/terminal.cc b/src/dev/serial/terminal.cc index fada99c..9564876 100644 --- a/src/dev/serial/terminal.cc +++ b/src/dev/serial/terminal.cc @@ -175,7 +175,7 @@ return; } - while (!listener.listen(port, true)) { + while (!listener.listen(port)) { DPRINTF(Terminal, ": can't bind address terminal port %d inuse PID %d\n", port, getpid()); -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/69159?usp=email To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings Gerrit-Project: public/gem5 Gerrit-Branch: develop Gerrit-Change-Id: Ie23971aabf2fe4252d27f1887468360722a72379 Gerrit-Change-Number: 69159 Gerrit-PatchSet: 3 Gerrit-Owner: Gabe Black <gabe.black@gmail.com> Gerrit-Reviewer: Bobby Bruce <bbruce@ucdavis.edu> Gerrit-Reviewer: Daniel Carvalho <odanrc@yahoo.com.br> Gerrit-Reviewer: Gabe Black <gabe.black@gmail.com> Gerrit-Reviewer: Gabe Black <gabeblack@google.com> Gerrit-Reviewer: Jason Lowe-Power <jason@lowepower.com> Gerrit-Reviewer: Yu-hsin Wang <yuhsingw@google.com> Gerrit-Reviewer: kokoro <noreply+kokoro@google.com> Gerrit-MessageType: merged