From 8899e006aab3e0831214d187596eef450b5e21b7 Mon Sep 17 00:00:00 2001 From: Peter Wu Date: Wed, 3 Feb 2016 18:28:46 +0100 Subject: [PATCH] Fix memleaks related to get_dirname get_dirname may return NULL instead of the original string, so avoid patterns like get_dirname(strdup(x)). Writing to cf_path.toUtf8().data() is fine btw, toUtf8() returns new memory. This fixes two memleak reported by LeakSanitizer via fileset_add_dir and MainWindow::captureFileReadFinished (both via cf_callback_invoke). Change-Id: I0f1528763e77e1f55b54b6674c890a9d02302ee8 Reviewed-on: https://code.wireshark.org/review/13691 Petri-Dish: Dario Lombardo Tested-by: Petri Dish Buildbot Reviewed-by: Anders Broman --- fileset.c | 4 ++-- ui/gtk/main.c | 12 ++++++------ ui/qt/main_window.cpp | 8 ++++---- ui/qt/main_window_slots.cpp | 6 +++--- wireshark-qt.cpp | 4 ++-- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/fileset.c b/fileset.c index 60bb6d5223..b1e636c3d5 100644 --- a/fileset.c +++ b/fileset.c @@ -275,8 +275,8 @@ fileset_add_dir(const char *fname, void *window) /* get (convert) directory name, but don't touch the given string */ - fname_dup = get_dirname(g_strdup(fname)); - dirname = g_string_new(fname_dup); + fname_dup = g_strdup(fname); + dirname = g_string_new(get_dirname(fname_dup)); g_free(fname_dup); set.dirname = g_strdup(dirname->str); diff --git a/ui/gtk/main.c b/ui/gtk/main.c index b5294bafa4..5ceaf00255 100644 --- a/ui/gtk/main.c +++ b/ui/gtk/main.c @@ -1452,8 +1452,8 @@ main_cf_cb_file_read_finished(capture_file *cf) add_menu_recent_capture_file(cf->filename); /* Remember folder for next Open dialog and save it in recent */ - dir_path = get_dirname(g_strdup(cf->filename)); - set_last_open_dir(dir_path); + dir_path = g_strdup(cf->filename); + set_last_open_dir(get_dirname(dir_path)); g_free(dir_path); } @@ -1474,8 +1474,8 @@ main_cf_cb_file_rescan_finished(capture_file *cf) add_menu_recent_capture_file(cf->filename); /* Remember folder for next Open dialog and save it in recent */ - dir_path = get_dirname(g_strdup(cf->filename)); - set_last_open_dir(dir_path); + dir_path = g_strdup(cf->filename); + set_last_open_dir(get_dirname(dir_path)); g_free(dir_path); } @@ -3299,8 +3299,8 @@ main(int argc, char *argv[]) if (global_capture_opts.save_file != NULL) { /* Save the directory name for future file dialogs. */ /* (get_dirname overwrites filename) */ - s = get_dirname(g_strdup(global_capture_opts.save_file)); - set_last_open_dir(s); + s = g_strdup(global_capture_opts.save_file); + set_last_open_dir(get_dirname(s)); g_free(s); } /* "-k" was specified; start a capture. */ diff --git a/ui/qt/main_window.cpp b/ui/qt/main_window.cpp index 2dbcd25074..8f43775486 100644 --- a/ui/qt/main_window.cpp +++ b/ui/qt/main_window.cpp @@ -1323,8 +1323,8 @@ void MainWindow::saveAsCaptureFile(capture_file *cf, bool must_support_comments, case CF_WRITE_OK: /* The save succeeded; we're done. */ /* Save the directory name for future file dialogs. */ - dirname = get_dirname(qstring_strdup(file_name)); /* Overwrites cf_name */ - set_last_open_dir(dirname); + dirname = qstring_strdup(file_name); /* Overwrites cf_name */ + set_last_open_dir(get_dirname(dirname)); g_free(dirname); /* If we discarded comments, redraw the packet list to reflect any packets that no longer have comments. */ @@ -1463,8 +1463,8 @@ void MainWindow::exportSelectedPackets() { case CF_WRITE_OK: /* The save succeeded; we're done. */ /* Save the directory name for future file dialogs. */ - dirname = get_dirname(qstring_strdup(file_name)); /* Overwrites cf_name */ - set_last_open_dir(dirname); + dirname = qstring_strdup(file_name); /* Overwrites cf_name */ + set_last_open_dir(get_dirname(dirname)); g_free(dirname); /* If we discarded comments, redraw the packet list to reflect any packets that no longer have comments. */ diff --git a/ui/qt/main_window_slots.cpp b/ui/qt/main_window_slots.cpp index 1881350e41..a12649910f 100644 --- a/ui/qt/main_window_slots.cpp +++ b/ui/qt/main_window_slots.cpp @@ -261,7 +261,7 @@ bool MainWindow::openCaptureFile(QString cf_path, QString read_filter, unsigned } break; } - // get_dirname overwrites its path. Hopefully this isn't a problem. + // get_dirname overwrites its path. wsApp->setLastOpenDir(get_dirname(cf_path.toUtf8().data())); main_ui_->statusBar->showExpert(); @@ -706,8 +706,8 @@ void MainWindow::captureFileReadFinished() { add_menu_recent_capture_file(capture_file_.capFile()->filename); /* Remember folder for next Open dialog and save it in recent */ - dir_path = get_dirname(g_strdup(capture_file_.capFile()->filename)); - wsApp->setLastOpenDir(dir_path); + dir_path = g_strdup(capture_file_.capFile()->filename); + wsApp->setLastOpenDir(get_dirname(dir_path)); g_free(dir_path); } diff --git a/wireshark-qt.cpp b/wireshark-qt.cpp index 1eaff2c89a..d4441e2c55 100644 --- a/wireshark-qt.cpp +++ b/wireshark-qt.cpp @@ -1410,8 +1410,8 @@ int main(int argc, char *argv[]) if (global_capture_opts.save_file != NULL) { /* Save the directory name for future file dialogs. */ /* (get_dirname overwrites filename) */ - gchar *s = get_dirname(g_strdup(global_capture_opts.save_file)); - set_last_open_dir(s); + gchar *s = g_strdup(global_capture_opts.save_file); + set_last_open_dir(get_dirname(s)); g_free(s); } /* "-k" was specified; start a capture. */