From c9e11a6fdcf28fae33abecfa538d388388f7597a Mon Sep 17 00:00:00 2001
From: Nick Bolton <nick@symless.com>
Date: Mon, 29 Jul 2024 21:51:01 +0100
Subject: [PATCH] Assert arg value `num > 0` for BSD socket poll unblock pipe
 (#7415)

* Move integ test to corredt place and re-enable tests

* Depend on `poll.h` (removes support for non-POSIX systems)

* Only build tests for each arch

* Move back to unit tests (poll is mocked)

* Better error message for valgrind not found

* Simplify dependency injection for BSD sockets poll test

* Improve test readability for BSD net poll

* Split out 2-in-1 test for `isAnyAddr`

* Stub out sleep function

* Improve coverage for pollSocket

* Use gmock ON_CALL instead of manual mock

* Remove unused function signature

* Use conventional deps struct instead of std functional

* Add test for socket data FD set to -1

* Add assertation for adding unblock pipe

* Use older style array alloc

* Less precision around `getNetworkDataForThread` value

* Use `ssize_t` for `ignore`

* Remove unused var

* Update ChangeLog

* Assert `n > 0`

* Add `num > 0` to top assert

* Update ChangeLog

* Only run assert test in debug
---
 ChangeLog                                           |  1 +
 src/lib/arch/unix/ArchNetworkBSD.cpp                |  6 ++----
 .../unittests/arch/unix/ArchNetworkBSDTests.cpp     | 13 +++++++++++++
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index a38a8aab26..04ce6042de 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -61,6 +61,7 @@ Enhancements:
 - #7412 Reduce GUI compile time by building a GUI library
 - #7413 Improve UI design and reduce over-use of `#ifdef`
 - #7414 Expand BSD sockets poll tests and remove legacy-poll code
+- #7415 Assert arg value `num > 0` for BSD socket poll unblock pipe
 
 # 1.14.6
 
diff --git a/src/lib/arch/unix/ArchNetworkBSD.cpp b/src/lib/arch/unix/ArchNetworkBSD.cpp
index 5465bbeef0..e69ae05dc4 100644
--- a/src/lib/arch/unix/ArchNetworkBSD.cpp
+++ b/src/lib/arch/unix/ArchNetworkBSD.cpp
@@ -269,7 +269,7 @@ bool ArchNetworkBSD::connectSocket(ArchSocket s, ArchNetAddress addr) {
 }
 
 int ArchNetworkBSD::pollSocket(PollEntry pe[], int num, double timeout) {
-  assert(pe != NULL || num == 0);
+  assert((pe != nullptr && num > 0) || num == 0);
 
   // return if nothing to do
   if (num == 0) {
@@ -299,9 +299,7 @@ int ArchNetworkBSD::pollSocket(PollEntry pe[], int num, double timeout) {
   // add the unblock pipe
   const int *unblockPipe = getUnblockPipe();
   if (unblockPipe != nullptr) {
-    assert(n < (1 + num));
-    pfd[n].fd = unblockPipe[0];
-
+    pfd[n].fd = unblockPipe[0]; // test
     pfd[n].events = POLLIN;
     ++n;
   }
diff --git a/src/test/unittests/arch/unix/ArchNetworkBSDTests.cpp b/src/test/unittests/arch/unix/ArchNetworkBSDTests.cpp
index 3145479c01..b9796f6820 100644
--- a/src/test/unittests/arch/unix/ArchNetworkBSDTests.cpp
+++ b/src/test/unittests/arch/unix/ArchNetworkBSDTests.cpp
@@ -48,6 +48,19 @@ struct MockDeps : public ArchNetworkBSD::Deps {
   std::shared_ptr<PollFD> m_pollFD;
 };
 
+#ifndef NDEBUG
+
+TEST(ArchNetworkBSDTests, pollSocket_negativeNum_death) {
+  MockDeps deps;
+  ArchNetworkBSD networkBSD(deps);
+
+  PollEntries entries{{nullptr, 0, 0}};
+
+  EXPECT_DEATH({ networkBSD.pollSocket(entries.data(), -1, 1); }, "num > 0");
+}
+
+#endif // DEBUG
+
 TEST(ArchNetworkBSDTests, pollSocket_zeroEntries_callsSleep) {
   MockDeps deps;
   ArchNetworkBSD networkBSD(deps);