Skip to content

Conversation

@sc0w
Copy link
Member

@sc0w sc0w commented Aug 15, 2020

No description provided.


wxString test(wxString(text, *wxConvCurrent).Lower());
while (nextCommand != curCommands->end()) {
while (curCommands && (nextCommand != curCommands->end())) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation for rl_completion_matches says that entry_func (i.e. command_completion() in this case) will first be called with state being zero on the first call, and non-zero on subsequent calls. Thus curCommands (and nextCommand, too) will always be initialized and non-zero on calls where state is non-zero.

}
} else {
if (!pClient->SupportsLargeFiles()) {
if (pClient && (!pClient->SupportsLargeFiles())) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The wxASSERT call on line 1093 properly checks for both pClient and pServer being non-null. However, only in debug builds (wxDEBUG enabled). That should be changed to something that works also in release builds (like wxCHECK_RET) and further checks can be omitted.

// - if the compressed size is still >= the original size, we send the uncompressed packet
// therefor we always try to compress the packet
if (server->GetTCPFlags() & SRV_TCPFLG_COMPRESSION){
if (server && (server->GetTCPFlags() & SRV_TCPFLG_COMPRESSION)){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First of all, this function should only be called when we're connected to a server. But just in case, if we are not connected (i.e. GetCurrentServer() returns NULL on line 707) we could bail out immediately, because SendPacket() will fail anyway at the end of the function. Of course that would render all further checks for server being non-null obsolete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants