verify if some objects are null - #212
Conversation
|
|
||
| wxString test(wxString(text, *wxConvCurrent).Lower()); | ||
| while (nextCommand != curCommands->end()) { | ||
| while (curCommands && (nextCommand != curCommands->end())) { |
There was a problem hiding this comment.
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())) { |
There was a problem hiding this comment.
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)){ |
There was a problem hiding this comment.
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.
|
I pushed a follow-up commit (5cc30c24) that addresses @gonosztopi's three inline review comments. ExternalConnector.cpp — The KnownFile.cpp — Instead of the inline SharedFileList.cpp — An early return has been added immediately after |
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.
|
The branch has been rebased onto |
All three review points have been addressed in the rebased commits.
|
@got3nks , I'd appreciate your review, in case I missed something. |
|
@mrjimenez — LGTM in spirit, one trailing cleanup. gonosztopi's review said the early return "would render all further checks for if (!file->IsLargeFile() || (server && server->SupportsLargeFilesTCP())) {After the early return, if (!file->IsLargeFile() || server->SupportsLargeFilesTCP()) {Defensive but redundant. Otherwise LGTM — happy to approve once this is in. |
|
Oops, misread the last line. I will open a new PR for the issue you raised. |
SendListToServer() already returns early when GetCurrentServer() yields NULL (added in the previous commit), so the `server &&` guard in the SupportsLargeFilesTCP() check is now unreachable with a null pointer and can be removed. Follows up on review feedback from got3nks on PR #212.
…mplate (amule-project#212) Add a master checkbox in the table header of the Downloads, Search and Shared pages that toggles every row checkbox at once, instead of having to tick files one by one. - Add a JS selectAll(check) helper to each page that walks all input[type=checkbox] in the document and sets them to check.checked. - Replace the empty header cell with <th class="al-left"> containing the master checkbox (onclick="selectAll(this)"). The al-left class matches the left alignment of the row checkbox cells, which <th> would otherwise center. - Name the master checkbox selectAllFiles (not a 32-char hash) so the command-processing loop, which only acts on 32-char file-hash names, ignores it automatically. No backend changes; all logic is client-side.
4 participants
No description provided.