From 570c6374806d0f1ec59cf7a72543efe6b5b637be Mon Sep 17 00:00:00 2001 From: Milan Crha Date: Fri, 15 Nov 2013 09:06:57 +0100 Subject: Fix/mute issues found by Coverity scan This makes the code free of Coverity scan issues. It is sometimes quite pedantic and expects/suggests some coding habits, thus certain changes may look weird, but for a good thing, I hope. The code is also tagged with Coverity scan suppressions, to keep the code as is and hide the warning too. Also note that Coverity treats g_return_if_fail(), g_assert() and similar macros as unreliable, and it's true these can be disabled during the compile time, thus it brings in other set of 'weird' changes. --- mail/e-http-request.c | 4 +- mail/e-mail-backend.c | 6 +- mail/e-mail-config-page.c | 1 + mail/e-mail-migrate.c | 9 +- mail/e-mail-reader-utils.c | 3 + mail/e-mail-reader.c | 1 - mail/em-composer-utils.c | 6 +- mail/em-filter-rule.c | 5 +- mail/em-folder-properties.c | 136 +++++++++++++++---------------- mail/em-folder-tree.c | 55 +++++++------ mail/em-utils.c | 8 +- mail/importers/evolution-mbox-importer.c | 28 +++++-- mail/importers/mail-importer.c | 2 + mail/message-list.c | 25 +++--- 14 files changed, 163 insertions(+), 126 deletions(-) (limited to 'mail') diff --git a/mail/e-http-request.c b/mail/e-http-request.c index 5a2601c0e3..c12fd76ca1 100644 --- a/mail/e-http-request.c +++ b/mail/e-http-request.c @@ -197,12 +197,12 @@ handle_http_request (GSimpleAsyncResult *res, evo_uri = soup_uri_to_string (soup_uri, FALSE); if (camel_debug_start ("emformat:requests")) { - printf ("%s: looking for '%s'\n", G_STRFUNC, evo_uri); + printf ("%s: looking for '%s'\n", G_STRFUNC, evo_uri ? evo_uri : "[null]"); camel_debug_end (); } /* Remove the "evo-" prefix from scheme */ - uri_len = strlen (evo_uri); + uri_len = evo_uri ? strlen (evo_uri) : 0; uri = NULL; if (evo_uri && (uri_len > 5)) { diff --git a/mail/e-mail-backend.c b/mail/e-mail-backend.c index bc30ad1d94..54e2ccbec7 100644 --- a/mail/e-mail-backend.c +++ b/mail/e-mail-backend.c @@ -28,6 +28,7 @@ #include "e-mail-backend.h" +#include #include #include #include @@ -638,7 +639,10 @@ mail_backend_folder_renamed_cb (MailFolderCache *folder_cache, newname = mail_backend_uri_to_evname (new_uri, cachenames[ii]); /* Ignore errors; doesn't matter. */ - g_rename (oldname, newname); + if (g_rename (oldname, newname) == -1) { + g_warning ("%s: Failed to rename '%s' to '%s': %s", G_STRFUNC, + oldname, newname, g_strerror (errno)); + } g_free (oldname); g_free (newname); diff --git a/mail/e-mail-config-page.c b/mail/e-mail-config-page.c index 54c71b1c0c..36f8c08835 100644 --- a/mail/e-mail-config-page.c +++ b/mail/e-mail-config-page.c @@ -191,6 +191,7 @@ e_mail_config_page_compare (GtkWidget *page_a, if (interface_a == NULL && interface_b != NULL) return 1; + /* coverity[var_deref_op] */ if (interface_a->sort_order < interface_b->sort_order) return -1; diff --git a/mail/e-mail-migrate.c b/mail/e-mail-migrate.c index e982b402ed..6a97b7342f 100644 --- a/mail/e-mail-migrate.c +++ b/mail/e-mail-migrate.c @@ -149,7 +149,9 @@ cp (const gchar *src, ut.actime = st.st_atime; ut.modtime = st.st_mtime; utime (dest, &ut); - chmod (dest, st.st_mode); + if (chmod (dest, st.st_mode) == -1) { + g_warning ("%s: Failed to chmod '%s': %s", G_STRFUNC, dest, g_strerror (errno)); + } return TRUE; @@ -264,7 +266,10 @@ em_rename_view_in_folder (gpointer data, oldname = g_build_filename (views_dir, filename, NULL); newname = g_build_filename (views_dir, newfile, NULL); - g_rename (oldname, newname); + if (g_rename (oldname, newname) == -1) { + g_warning ("%s: Failed to rename '%s' to '%s': %s", G_STRFUNC, + oldname, newname, g_strerror (errno)); + } g_checksum_free (checksum); g_free (oldname); diff --git a/mail/e-mail-reader-utils.c b/mail/e-mail-reader-utils.c index 78f442d806..99a86a1f12 100644 --- a/mail/e-mail-reader-utils.c +++ b/mail/e-mail-reader-utils.c @@ -553,6 +553,9 @@ mail_reader_refresh_folder_cb (GObject *source_object, GError *local_error = NULL; folder = CAMEL_FOLDER (source_object); + if (!camel_folder_refresh_info_finish (folder, result, &local_error) && !local_error) + local_error = g_error_new_literal (CAMEL_ERROR, CAMEL_ERROR_GENERIC, _("Unknown error")); + async_context = (AsyncContext *) user_data; activity = async_context->activity; diff --git a/mail/e-mail-reader.c b/mail/e-mail-reader.c index 9fbed51270..63ab511b0e 100644 --- a/mail/e-mail-reader.c +++ b/mail/e-mail-reader.c @@ -2943,7 +2943,6 @@ mail_reader_set_folder (EMailReader *reader, priv = E_MAIL_READER_GET_PRIVATE (reader); - backend = e_mail_reader_get_backend (reader); display = e_mail_reader_get_mail_display (reader); message_list = e_mail_reader_get_message_list (reader); diff --git a/mail/em-composer-utils.c b/mail/em-composer-utils.c index 62405fdb5b..1b608906be 100644 --- a/mail/em-composer-utils.c +++ b/mail/em-composer-utils.c @@ -468,9 +468,10 @@ composer_presend_check_unwanted_html (EMsgComposer *composer, gboolean html_problem = FALSE; for (ii = 0; recipients[ii] != NULL; ii++) { - if (!e_destination_get_html_mail_pref (recipients[ii])) + if (!e_destination_get_html_mail_pref (recipients[ii])) { html_problem = TRUE; break; + } } if (html_problem) { @@ -2882,14 +2883,13 @@ em_utils_construct_composer_text (CamelSession *session, EMailPartList *parts_list) { gchar *text, *credits; - gboolean start_bottom = 0; g_return_val_if_fail (CAMEL_IS_SESSION (session), NULL); credits = attribution_format (message); text = em_utils_message_to_html ( session, message, credits, E_MAIL_FORMATTER_QUOTE_FLAG_CITE, - parts_list, start_bottom ? "
" : NULL, NULL); + parts_list, NULL, NULL); g_free (credits); return text; diff --git a/mail/em-filter-rule.c b/mail/em-filter-rule.c index 595bef8bfe..8bd1f6d332 100644 --- a/mail/em-filter-rule.c +++ b/mail/em-filter-rule.c @@ -310,7 +310,10 @@ part_combobox_changed (GtkComboBox *combobox, /* traverse until reached index */ } - g_return_if_fail (part != NULL); + if (!part) { + g_warn_if_reached (); + return; + } g_return_if_fail (i == index); /* dont update if we haven't changed */ diff --git a/mail/em-folder-properties.c b/mail/em-folder-properties.c index d0ec478dd5..e7342b7869 100644 --- a/mail/em-folder-properties.c +++ b/mail/em-folder-properties.c @@ -148,6 +148,18 @@ emfp_get_folder_item (EConfig *ec, guint ii, n_properties; gint row = 0; gboolean can_apply_filters = FALSE; + CamelStore *store; + CamelSession *session; + CamelFolderInfoFlags fi_flags = 0; + const gchar *folder_name; + MailFolderCache *folder_cache; + gboolean have_flags; + ESourceRegistry *registry; + EShell *shell; + EMailBackend *mail_backend; + EMailSendAccountOverride *account_override; + gchar *folder_uri, *account_uid; + GtkWidget *label; if (old) return old; @@ -210,32 +222,23 @@ emfp_get_folder_item (EConfig *ec, } } - if (context->folder != NULL) { - CamelStore *store; - CamelSession *session; - CamelFolderInfoFlags fi_flags = 0; - const gchar *folder_name; - MailFolderCache *folder_cache; - gboolean have_flags; - - store = camel_folder_get_parent_store (context->folder); - folder_name = camel_folder_get_full_name (context->folder); + store = camel_folder_get_parent_store (context->folder); + folder_name = camel_folder_get_full_name (context->folder); - session = camel_service_ref_session (CAMEL_SERVICE (store)); + session = camel_service_ref_session (CAMEL_SERVICE (store)); - folder_cache = e_mail_session_get_folder_cache ( - E_MAIL_SESSION (session)); + folder_cache = e_mail_session_get_folder_cache ( + E_MAIL_SESSION (session)); - have_flags = mail_folder_cache_get_folder_info_flags ( - folder_cache, store, folder_name, &fi_flags); + have_flags = mail_folder_cache_get_folder_info_flags ( + folder_cache, store, folder_name, &fi_flags); - can_apply_filters = - !CAMEL_IS_VEE_FOLDER (context->folder) && - have_flags && - (fi_flags & CAMEL_FOLDER_TYPE_MASK) != CAMEL_FOLDER_TYPE_INBOX; + can_apply_filters = + !CAMEL_IS_VEE_FOLDER (context->folder) && + have_flags && + (fi_flags & CAMEL_FOLDER_TYPE_MASK) != CAMEL_FOLDER_TYPE_INBOX; - g_object_unref (session); - } + g_object_unref (session); class = G_OBJECT_GET_CLASS (context->folder); properties = g_object_class_list_properties (class, &n_properties); @@ -277,56 +280,47 @@ emfp_get_folder_item (EConfig *ec, g_free (properties); /* add send-account-override setting widgets */ - if (context->folder != NULL) { - ESourceRegistry *registry; - EShell *shell; - EMailBackend *mail_backend; - EMailSendAccountOverride *account_override; - gchar *folder_uri, *account_uid; - GtkWidget *label; - - registry = e_shell_get_registry (e_shell_get_default ()); - - label = gtk_label_new_with_mnemonic (_("_Send Account Override:")); - gtk_widget_set_halign (label, GTK_ALIGN_START); - gtk_widget_show (label); - gtk_table_attach ( - GTK_TABLE (table), label, - 0, 2, row, row + 1, - GTK_FILL, 0, 0, 0); - row++; - - widget = g_object_new ( - E_TYPE_MAIL_IDENTITY_COMBO_BOX, - "registry", registry, - "allow-none", TRUE, - NULL); - gtk_label_set_mnemonic_widget (GTK_LABEL (label), widget); - gtk_widget_set_margin_left (widget, 12); - gtk_widget_show (widget); - gtk_table_attach ( - GTK_TABLE (table), widget, - 0, 2, row, row + 1, - GTK_FILL | GTK_EXPAND, 0, 0, 0); - row++; - - shell = e_shell_get_default (); - mail_backend = E_MAIL_BACKEND (e_shell_get_backend_by_name (shell, "mail")); - g_return_val_if_fail (mail_backend != NULL, table); - - account_override = e_mail_backend_get_send_account_override (mail_backend); - folder_uri = e_mail_folder_uri_from_folder (context->folder); - account_uid = e_mail_send_account_override_get_for_folder (account_override, folder_uri); - - gtk_combo_box_set_active_id (GTK_COMBO_BOX (widget), account_uid ? account_uid : ""); - g_object_set_data_full (G_OBJECT (widget), "sao-folder-uri", folder_uri, g_free); - - g_signal_connect ( - widget, "changed", - G_CALLBACK (mail_identity_combo_box_changed_cb), account_override); - - g_free (account_uid); - } + registry = e_shell_get_registry (e_shell_get_default ()); + + label = gtk_label_new_with_mnemonic (_("_Send Account Override:")); + gtk_widget_set_halign (label, GTK_ALIGN_START); + gtk_widget_show (label); + gtk_table_attach ( + GTK_TABLE (table), label, + 0, 2, row, row + 1, + GTK_FILL, 0, 0, 0); + row++; + + widget = g_object_new ( + E_TYPE_MAIL_IDENTITY_COMBO_BOX, + "registry", registry, + "allow-none", TRUE, + NULL); + gtk_label_set_mnemonic_widget (GTK_LABEL (label), widget); + gtk_widget_set_margin_left (widget, 12); + gtk_widget_show (widget); + gtk_table_attach ( + GTK_TABLE (table), widget, + 0, 2, row, row + 1, + GTK_FILL | GTK_EXPAND, 0, 0, 0); + row++; + + shell = e_shell_get_default (); + mail_backend = E_MAIL_BACKEND (e_shell_get_backend_by_name (shell, "mail")); + g_return_val_if_fail (mail_backend != NULL, table); + + account_override = e_mail_backend_get_send_account_override (mail_backend); + folder_uri = e_mail_folder_uri_from_folder (context->folder); + account_uid = e_mail_send_account_override_get_for_folder (account_override, folder_uri); + + gtk_combo_box_set_active_id (GTK_COMBO_BOX (widget), account_uid ? account_uid : ""); + g_object_set_data_full (G_OBJECT (widget), "sao-folder-uri", folder_uri, g_free); + + g_signal_connect ( + widget, "changed", + G_CALLBACK (mail_identity_combo_box_changed_cb), account_override); + + g_free (account_uid); return table; } diff --git a/mail/em-folder-tree.c b/mail/em-folder-tree.c index 27091e2f5c..aaf3488307 100644 --- a/mail/em-folder-tree.c +++ b/mail/em-folder-tree.c @@ -554,6 +554,8 @@ folder_tree_maybe_expand_row (EMFolderTreeModel *model, if (u) { gchar *c = strrchr (key, '/'); + /* 'c' cannot be NULL, because the above contructed 'key' has it there */ + /* coverity[dereference] */ *c = '\0'; folder_tree_expand_node (key, folder_tree); @@ -3036,12 +3038,10 @@ em_folder_tree_set_selected_list (EMFolderTree *folder_tree, g_slist_append (priv->select_uris, u); } - end = strrchr (expand_key, '/'); - do { + while (end = strrchr (expand_key, '/'), end) { folder_tree_expand_node (expand_key, folder_tree); *end = 0; - end = strrchr (expand_key, '/'); - } while (end); + } if (expand_only) folder_tree_free_select_uri (u); @@ -3108,33 +3108,39 @@ em_folder_tree_select_next_path (EMFolderTree *folder_tree, current_path = gtk_tree_model_get_path (model, &iter); do { - if (gtk_tree_model_iter_has_child (model, &iter)) { - gtk_tree_model_iter_children (model, &child, &iter); - path = gtk_tree_model_get_path (model, &child); - iter = child; - } else { - while (1) { - gboolean has_parent; + if (gtk_tree_model_iter_has_child (model, &iter)) { + if (!gtk_tree_model_iter_children (model, &child, &iter)) + break; + path = gtk_tree_model_get_path (model, &child); + iter = child; + } else { + while (1) { + gboolean has_parent; - has_parent = gtk_tree_model_iter_parent ( - model, &parent, &iter); + has_parent = gtk_tree_model_iter_parent ( + model, &parent, &iter); - if (gtk_tree_model_iter_next (model, &iter)) { - path = gtk_tree_model_get_path (model, &iter); - break; - } else { - if (has_parent) { - iter = parent; - } else { - /* Reached end. Wrapup*/ - gtk_tree_model_get_iter_first (model, &iter); + if (gtk_tree_model_iter_next (model, &iter)) { path = gtk_tree_model_get_path (model, &iter); break; + } else { + if (has_parent) { + iter = parent; + } else { + /* Reached end. Wrapup*/ + if (gtk_tree_model_get_iter_first (model, &iter)) + path = gtk_tree_model_get_path (model, &iter); + else + path = NULL; + break; + } } } + + if (!path) + break; } - } - gtk_tree_model_get (model, &iter, COL_UINT_UNREAD, &unread, -1); + gtk_tree_model_get (model, &iter, COL_UINT_UNREAD, &unread, -1); /* TODO : Flags here for better options */ } while (skip_read_folders && unread <=0 && @@ -3153,6 +3159,7 @@ em_folder_tree_select_next_path (EMFolderTree *folder_tree, } gtk_tree_view_scroll_to_cell (tree_view, path, NULL, TRUE, 0.5f, 0.0f); } + return; } diff --git a/mail/em-utils.c b/mail/em-utils.c index db68b96003..6669ef5aff 100644 --- a/mail/em-utils.c +++ b/mail/em-utils.c @@ -590,7 +590,6 @@ em_utils_read_messages_from_stream (CamelFolder *folder, while (camel_mime_parser_step (mp, NULL, NULL) == CAMEL_MIME_PARSER_STATE_FROM) { CamelMimeMessage *msg; - gboolean success; /* NB: de-from filter, once written */ msg = camel_mime_message_new (); @@ -1060,6 +1059,8 @@ em_utils_selection_set_urilist (GtkSelectionData *data, exit: g_free (tmpdir); + /* the 'fd' from the 'save_as_mbox' part is freed within the 'fstream' */ + /* coverity[leaked_handle] */ } /** @@ -1094,6 +1095,8 @@ em_utils_selection_get_urilist (GtkSelectionData *selection_data, if (url == NULL) continue; + /* 'fd', if set, is freed within the 'stream' */ + /* coverity[overwrite_var] */ if (strcmp (url->protocol, "file") == 0 && (fd = g_open (url->path, O_RDONLY | O_BINARY, 0)) != -1) { stream = camel_stream_fs_new_with_fd (fd); @@ -1107,6 +1110,9 @@ em_utils_selection_get_urilist (GtkSelectionData *selection_data, } g_strfreev (uris); + + /* 'fd', if set, is freed within the 'stream' */ + /* coverity[leaked_handle] */ } /* ********************************************************************** */ diff --git a/mail/importers/evolution-mbox-importer.c b/mail/importers/evolution-mbox-importer.c index 5eb59e82f0..eb5746a746 100644 --- a/mail/importers/evolution-mbox-importer.c +++ b/mail/importers/evolution-mbox-importer.c @@ -374,8 +374,7 @@ mbox_get_preview (EImport *ei, mp = camel_mime_parser_new (); camel_mime_parser_scan_from (mp, TRUE); if (camel_mime_parser_init_with_fd (mp, fd) == -1) { - g_object_unref (mp); - return NULL; + goto cleanup; } while (camel_mime_parser_step (mp, NULL, NULL) == CAMEL_MIME_PARSER_STATE_FROM) { @@ -416,14 +415,18 @@ mbox_get_preview (EImport *ei, if (store) { GtkTreeView *tree_view; GtkTreeSelection *selection; - gboolean valid; preview = e_web_view_preview_new (); gtk_widget_show (preview); tree_view = e_web_view_preview_get_tree_view ( E_WEB_VIEW_PREVIEW (preview)); - g_return_val_if_fail (tree_view != NULL, NULL); + if (!tree_view) { + g_warn_if_reached (); + gtk_widget_destroy (preview); + preview = NULL; + goto cleanup; + } gtk_tree_view_set_model (tree_view, GTK_TREE_MODEL (store)); g_object_unref (store); @@ -443,16 +446,20 @@ mbox_get_preview (EImport *ei, E_WEB_VIEW_PREVIEW (preview)); create_preview_func (G_OBJECT (preview), &preview_widget); - g_return_val_if_fail (preview_widget != NULL, NULL); + if (!preview_widget) { + g_warn_if_reached (); + goto cleanup; + } e_web_view_preview_set_preview ( E_WEB_VIEW_PREVIEW (preview), preview_widget); gtk_widget_show (preview_widget); selection = gtk_tree_view_get_selection (tree_view); - valid = gtk_tree_model_get_iter_first ( - GTK_TREE_MODEL (store), &iter); - g_return_val_if_fail (valid, NULL); + if (!gtk_tree_model_get_iter_first (GTK_TREE_MODEL (store), &iter)) { + g_warn_if_reached (); + goto cleanup; + } gtk_tree_selection_select_iter (selection, &iter); g_signal_connect ( @@ -463,6 +470,11 @@ mbox_get_preview (EImport *ei, selection, E_WEB_VIEW_PREVIEW (preview)); } + cleanup: + g_object_unref (mp); + + /* 'fd' is freed together with 'mp' */ + /* coverity[leaked_handle] */ return preview; } diff --git a/mail/importers/mail-importer.c b/mail/importers/mail-importer.c index 0af34a3861..ac432b58ef 100644 --- a/mail/importers/mail-importer.c +++ b/mail/importers/mail-importer.c @@ -203,6 +203,8 @@ fail1: /* FIXME Not passing a GCancellable or GError here. */ camel_folder_synchronize_sync (folder, FALSE, NULL, NULL); g_object_unref (folder); + /* 'fd' is freed together with 'mp' */ + /* coverity[leaked_handle] */ } static void diff --git a/mail/message-list.c b/mail/message-list.c index 89a473a7ff..b54af0fe68 100644 --- a/mail/message-list.c +++ b/mail/message-list.c @@ -1233,7 +1233,7 @@ thread_select_foreach (ETreePath path, do { last = node; node = node->parent; - } while (!G_NODE_IS_ROOT (node)); + } while (node && !G_NODE_IS_ROOT (node)); g_ptr_array_add (tsi->paths, last); @@ -1719,14 +1719,14 @@ ml_tree_value_at_ex (ETreeModel *etm, /* Extract the single label from the hashtable. */ g_hash_table_iter_init (&iter, ld.labels_tag2iter); - g_hash_table_iter_next (&iter, NULL, (gpointer *) &label_defn); + if (g_hash_table_iter_next (&iter, NULL, (gpointer *) &label_defn)) { + e_mail_label_list_store_get_color (ld.store, label_defn, &colour_val); - e_mail_label_list_store_get_color (ld.store, label_defn, &colour_val); - - /* XXX Hack to avoid returning an allocated string. */ - colour_alloced = gdk_color_to_string (&colour_val); - colour = g_intern_string (colour_alloced); - g_free (colour_alloced); + /* XXX Hack to avoid returning an allocated string. */ + colour_alloced = gdk_color_to_string (&colour_val); + colour = g_intern_string (colour_alloced); + g_free (colour_alloced); + } } else if (camel_message_info_flags (msg_info) & CAMEL_MESSAGE_FLAGGED) { /* FIXME: extract from the important.xpm somehow. */ colour = "#A7453E"; @@ -5412,9 +5412,11 @@ message_list_regen_thread (GSimpleAsyncResult *simple, /* Handle search error or cancellation. */ - if (local_error == NULL) + if (local_error == NULL) { + /* coverity[unchecked_value] */ g_cancellable_set_error_if_cancelled ( cancellable, &local_error); + } if (local_error != NULL) { g_simple_async_result_take_error (simple, local_error); @@ -5506,9 +5508,8 @@ message_list_regen_done_cb (GObject *source_object, activity = regen_data->activity; - g_simple_async_result_propagate_error (simple, &local_error); - - if (e_activity_handle_cancellation (activity, local_error)) { + if (g_simple_async_result_propagate_error (simple, &local_error) && + e_activity_handle_cancellation (activity, local_error)) { g_error_free (local_error); return; -- cgit v1.2.3