Address reviewer feedback on null-pointer guards

mrjimenez · mrjimenez · commit 5cc30c24daf2 · 2026-06-07T14:49:59.000-03:00
ExternalConnector.cpp: remove redundant curCommands null guard; the
else branch already returns NULL when curCommands is NULL, so the
while loop is unreachable with a null pointer.

KnownFile.cpp: replace wxASSERT with wxCHECK_RET so the mutual-
exclusion check on pClient/pServer fires in release builds too, not
only in debug. Drop the redundant pClient null guard added by the PR;
the contract enforced at the function entry is sufficient.

SharedFileList.cpp: bail out early when GetCurrentServer() returns
NULL instead of guarding individual uses of server inline. The inline
null guard added by the PR is removed since it is now covered by the
early return.
diff --git a/src/ExternalConnector.cpp b/src/ExternalConnector.cpp
@@ -244,7 +244,7 @@ char *command_completion(const char *text, int state)
 	}
 
 	wxString test(wxString(text, *wxConvCurrent).Lower());
-	while (curCommands && (nextCommand != curCommands->end())) {
+	while (nextCommand != curCommands->end()) {
 		wxString curTest = (*nextCommand)->GetCommand();
 		++nextCommand;
 		if (curTest.Lower().StartsWith(test)) {
diff --git a/src/KnownFile.cpp b/src/KnownFile.cpp
@@ -1090,7 +1090,7 @@ void CKnownFile::CreateOfferedFilePacket(
 	// shared files to some other client. In each case we send our IP+Port only, if
 	// we have a HighID.
 
-	wxASSERT(!(pClient && pServer));
+	wxCHECK_RET(!(pClient && pServer), "pClient and pServer cannot both be non-null");
 
 	SetPublishedED2K(true);
 	files->WriteHash(GetFileHash());
@@ -1153,7 +1153,7 @@ void CKnownFile::CreateOfferedFilePacket(
 					tags.push_back(new CTagInt32(FT_FILESIZE_HI, (uint32)(GetFileSize() >> 32)));
 				}
 			} else {
-				if (pClient && (!pClient->SupportsLargeFiles())) {
+				if (!pClient->SupportsLargeFiles()) {
 					wxFAIL;
 					tags.push_back(new CTagInt32(FT_FILESIZE, 0));
 				} else {
diff --git a/src/SharedFileList.cpp b/src/SharedFileList.cpp
@@ -705,8 +705,11 @@ void CSharedFileList::SendListToServer(){
 	// Limits for the server.
 
 	CServer* server = theApp->serverconnect->GetCurrentServer();
+	if (!server) {
+		return;
+	}
 
-	uint32 limit = server ? server->GetSoftFiles() : 0;
+	uint32 limit = server->GetSoftFiles();
 	if( limit == 0 || limit > 200 ) {
 		limit = 200;
 	}
@@ -745,7 +748,7 @@ void CSharedFileList::SendListToServer(){
 	//   - this function is called once when connecting to a server and when a file becomes shareable - so, it's called rarely.
 	//   - if the compressed size is still >= the original size, we send the uncompressed packet
 	// therefor we always try to compress the packet
-	if (server && (server->GetTCPFlags() & SRV_TCPFLG_COMPRESSION)){
+	if (server->GetTCPFlags() & SRV_TCPFLG_COMPRESSION){
 		packet->PackPacket();
 	}
 

Original file line number	Diff line number	Diff line change
`@@ -244,7 +244,7 @@ char command_completion(const char text, int state)`
`244`	`244`	`}`
`245`	`245`
`246`	`246`	`wxString test(wxString(text, *wxConvCurrent).Lower());`
`247`		`- while (curCommands && (nextCommand != curCommands->end())) {`
	`247`	`+ while (nextCommand != curCommands->end()) {`
`248`	`248`	`wxString curTest = (*nextCommand)->GetCommand();`
`249`	`249`	`++nextCommand;`
`250`	`250`	`if (curTest.Lower().StartsWith(test)) {`