GNOME Bugzilla – Bug 749190
Minor fixes for dead code and redundant variable checks
Last modified: 2015-05-11 23:08:46 UTC
All spotted by Coverity in its latest run. Some of these fixes will produce user-visible changes to error reporting (where previously that error reporting was broken). Some of them simply remove dead code — these need review by someone who knows what the original intention of that code was, since I am not sure whether the dead code was meant to be used and is instead being masked by a bug elsewhere which is not setting up the right conditions for the dead code to be used. If you need access to Coverity to see its control flow analysis, please let me know!
Created attachment 303172 [details] [review] edataserver: Fix error reporting in ESource credentials method In e_source_unset_last_credentials_required_arguments_sync(); similarly in e_source_get_last_credentials_required_arguments_sync(). Spotted by Coverity. CID: #1297157, #1296550
Created attachment 303173 [details] [review] calendar: Remove dead code in HTTP backend The HTTP status cannot be SOUP_STATUS_SSL_FAILED inside the SOUP_STATUS_IS_REDIRECTION block. Spotted by Coverity. CID: #1296551
Created attachment 303174 [details] [review] edataserver: Remove unused code from EFreeFormExp At that point in the control flow, word is always NULL, hence checking it is a no-op. This might actually be a bug further up in the code — I do not know enough about this function to be sure. Spotted by Coverity. CID: #1296549
Created attachment 303175 [details] [review] edataserverui: Remove redundant check for an error being set There is no way this loop can have local_error set before checking whether it is NULL. Even if the declaration of local_error was hoisted to the parent scope, all control paths which lead to the loop being entered would have local_error set to NULL. This might not be the correct fix, but there is certainly a problem somewhere in this code. Spotted by Coverity. CID: #1296548
Created attachment 303176 [details] [review] edataserver: Remove redundant code path from e_source_dup_secret_label() Further up in the code, backend_name is cleared if (type == NULL), leaving the possible options as: 1. (backend_name != NULL && type != NULL) 2. (backend_name == NULL && type != NULL) 3. (backend_name == NULL && type == NULL) Spotted by Coverity. CID: #1296547
Review of attachment 303172 [details] [review]: Right, I just made this same fix for 3.17.2+ (commit 2a755c34ea) and 3.16.3+ (commit 4571a71ab4).
Review of attachment 303173 [details] [review]: Looks fine, feel free to commit.
Review of attachment 303174 [details] [review]: Let's cleanup the code a bit. (I'll do it.)
(In reply to Milan Crha from comment #8) > Let's cleanup the code a bit. (I'll do it.) Created commit 756ccc0 in eds master (3.17.2+) Created commit 581b100 in eds gnome-3-16 (3.16.3+)
Review of attachment 303175 [details] [review]: Looks fine, feel free to commit.
Review of attachment 303176 [details] [review]: Looks fine, feel free to commit.
Thanks for the fast reviews and fixes. All pushed to master. Attachment 303173 [details] pushed as fdd7ef0 - calendar: Remove dead code in HTTP backend Attachment 303175 [details] pushed as 6ec4694 - edataserverui: Remove redundant check for an error being set Attachment 303176 [details] pushed as 7014b0f - edataserver: Remove redundant code path from e_source_dup_secret_label()