Address review comments

fdbrpc/IPAllowList.cpp

@@ -50,6 +50,11 @@ int netmaskWeightImpl(std::array<unsigned char, Sz> const& addr) {
 
 } // namespace
 
+AuthAllowedSubnet::AuthAllowedSubnet(IPAddress const& baseAddress, IPAddress const& addressMask)
+  : baseAddress(baseAddress), addressMask(addressMask) {
+	ASSERT(baseAddress.isV4() == addressMask.isV4());
+}
+
 IPAddress AuthAllowedSubnet::netmask() const {
 	if (addressMask.isV4()) {
 		uint32_t res = 0xffffffff ^ addressMask.toV4();
@@ -110,6 +115,30 @@ void AuthAllowedSubnet::printIP(std::string_view txt, IPAddress const& address)
 	fmt::print("\n");
 }
 
+template <std::size_t sz>
+std::array<unsigned char, sz> AuthAllowedSubnet::createBitMask(std::array<unsigned char, sz> const& addr,
+                                                               int netmaskWeight) {
+	std::array<unsigned char, sz> res;
+	res.fill((unsigned char)0xff);
+	int idx = netmaskWeight / 8;
+	if (netmaskWeight % 8 > 0) {
+		// 2^(netmaskWeight % 8) - 1 sets the last (netmaskWeight % 8) number of bits to 1
+		// everything else will be zero. For example: 2^3 - 1 == 7 == 0b111
+		unsigned char bitmask = (1 << (8 - (netmaskWeight % 8))) - ((unsigned char)1);
+		res[idx] ^= bitmask;
+		++idx;
+	}
+	for (; idx < res.size(); ++idx) {
+		res[idx] = (unsigned char)0;
+	}
+	return res;
+}
+
+template std::array<unsigned char, 4> AuthAllowedSubnet::createBitMask<4>(const std::array<unsigned char, 4>& addr,
+                                                                          int netmaskWeight);
+template std::array<unsigned char, 16> AuthAllowedSubnet::createBitMask<16>(const std::array<unsigned char, 16>& addr,
+                                                                            int netmaskWeight);
+
 // helpers for testing
 namespace {
 using boost::asio::ip::address_v4;

fdbrpc/IPAllowList.h

@@ -29,31 +29,12 @@ struct AuthAllowedSubnet {
 	IPAddress baseAddress;
 	IPAddress addressMask;
 
-	AuthAllowedSubnet(IPAddress const& baseAddress, IPAddress const& addressMask)
+	AuthAllowedSubnet(IPAddress const& baseAddress, IPAddress const& addressMask);
-	  : baseAddress(baseAddress), addressMask(addressMask) {
-		ASSERT(baseAddress.isV4() == addressMask.isV4());
-	}
 
 	static AuthAllowedSubnet fromString(std::string_view addressString);
 
 	template <std::size_t sz>
-	static auto createBitMask(std::array<unsigned char, sz> const& addr, int netmaskWeight)
+	static std::array<unsigned char, sz> createBitMask(std::array<unsigned char, sz> const& addr, int netmaskWeight);
-	    -> std::array<unsigned char, sz> {
-		std::array<unsigned char, sz> res;
-		res.fill((unsigned char)0xff);
-		int idx = netmaskWeight / 8;
-		if (netmaskWeight % 8 > 0) {
-			// 2^(netmaskWeight % 8) - 1 sets the last (netmaskWeight % 8) number of bits to 1
-			// everything else will be zero. For example: 2^3 - 1 == 7 == 0b111
-			unsigned char bitmask = (1 << (8 - (netmaskWeight % 8))) - ((unsigned char)1);
-			res[idx] ^= bitmask;
-			++idx;
-		}
-		for (; idx < res.size(); ++idx) {
-			res[idx] = (unsigned char)0;
-		}
-		return res;
-	}
 
 	bool operator()(IPAddress const& address) const {
 		if (addressMask.isV4() != address.isV4()) {

fdbserver/workloads/ClientWorkload.actor.cpp

@@ -56,13 +56,12 @@ class WorkloadProcessState {
 		} else {
 			self->childAddress = IPAddress::parse(fmt::format("192.168.0.{}", self->clientId + 2)).get();
 		}
-		//TraceEvent("TestClientStart", id).detail("ClusterFileLocation", child->ccr->getLocation()).log();
 		self->processName = fmt::format("TestClient{}", self->clientId);
 		Standalone<StringRef> newZoneId(deterministicRandom()->randomUniqueID().toString());
 		auto locality = LocalityData(Optional<Standalone<StringRef>>(), newZoneId, newZoneId, parent->locality.dcId());
 		auto dataFolder = joinPath(popPath(parent->dataFolder), deterministicRandom()->randomUniqueID().toString());
 		platform::createDirectory(dataFolder);
-		TraceEvent("StartingClientForWorkload", self->id)
+		TraceEvent("StartingClientWorkloadProcess", self->id)
 		    .detail("Name", self->processName)
 		    .detail("Address", self->childAddress);
 		self->childProcess = g_simulator.newProcess(self->processName.c_str(),
@@ -83,7 +82,7 @@ class WorkloadProcessState {
 			auto addr = g_simulator.getCurrentProcess()->address;
 			futures.push_back(FlowTransport::transport().bind(addr, addr));
 			futures.push_back(success((self->childProcess->onShutdown())));
-			TraceEvent("WorkloadProcessInitialized", self->id).log();
+			TraceEvent("ClientWorkloadProcessInitialized", self->id).log();
 			futures.push_back(initializationDone(self, parent));
 			wait(waitForAny(futures));
 		} catch (Error& e) {
@@ -96,28 +95,19 @@ class WorkloadProcessState {
 		return Void();
 	}
 
-	static std::vector<std::pair<WorkloadProcessState*, int>>& states() {
+	static std::vector<WorkloadProcessState*>& states() {
-		static std::vector<std::pair<WorkloadProcessState*, int>> res;
+		static std::vector<WorkloadProcessState*> res;
 		return res;
 	}
 
 public:
 	static WorkloadProcessState* instance(int clientId) {
-		states().resize(std::max(states().size(), size_t(clientId + 1)), std::make_pair(nullptr, 0));
+		states().resize(std::max(states().size(), size_t(clientId + 1)), nullptr);
 		auto& res = states()[clientId];
-		if (res.first == nullptr) {
+		if (res == nullptr) {
-			res = std::make_pair(new WorkloadProcessState(clientId), 0);
+			res = new WorkloadProcessState(clientId);
-		}
-		++res.second;
-		return res.first;
-	}
-	static void done(WorkloadProcessState* processState) {
-		auto& p = states()[processState->clientId];
-		ASSERT(p.first == processState);
-		if (--p.second == 0) {
-			// delete p.first;
-			// p.first = nullptr;
 		}
+		return res;
 	}
 
 	Future<Void> initialized() const { return init.getFuture(); }
@@ -201,7 +191,7 @@ struct WorkloadProcess {
 			res = r;
 		} catch (Error& e) {
 			if (e.code() == error_code_actor_cancelled) {
-				ASSERT(false);
+				ASSERT(g_simulator.getCurrentProcess() == parent);
 				throw;
 			}
 			err = e;
@@ -212,8 +202,6 @@ struct WorkloadProcess {
 		}
 		return res;
 	}
-
-	~WorkloadProcess() { WorkloadProcessState::done(processState); }
 };
 
 ClientWorkload::ClientWorkload(CreateWorkload const& childCreator, WorkloadContext const& wcx)

fdbserver/workloads/PrivateEndpoints.actor.cpp

@@ -59,23 +59,20 @@ struct PrivateEndpoints : TestWorkload {
 			T t = wait(f);
 			(void)t;
 			ASSERT(false);
-			return Void();
 		} catch (Error& e) {
 			if (e.code() == error_code_actor_cancelled) {
 				throw;
 			} else if (e.code() == error_code_unauthorized_attempt) {
 				TraceEvent("SuccessPrivateEndpoint").log();
-				return Void();
 			} else if (e.code() == error_code_request_maybe_delivered) {
 				// this is also fine, because even when calling private endpoints
 				// we might see connection failures
 				TraceEvent("SuccessRequestMaybeDelivered").log();
-				return Void();
 			} else {
 				TraceEvent(SevError, "WrongErrorCode").error(e);
-				return Void();
 			}
 		}
+		return Void();
 	}
 
 	template <class I, class RT>