[gem5-dev] [M] Change in gem5/gem5[develop]: base,cpu,dev: Simplify ListenSocket::listen().
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
Reviewed-by: Yu-hsin Wang
Tested-by: kokoro
---
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 expect
[gem5-dev] [M] Change in gem5/gem5[develop]: base,cpu,dev: Simplify ListenSocket::listen().
Gabe Black has uploaded this change for review. (
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
---
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(-)
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
