From d1873dbcc89eb3138027d8e5c5e4c5b12420e797 Mon Sep 17 00:00:00 2001 From: Hadriel Kaplan Date: Thu, 20 Mar 2014 12:57:29 -0400 Subject: [PATCH] Fix Bug 9903: 'Clicking reload-file ignores selected file format reader' There's a relatively new feature in 1.11.3 to select a specific file format reader, instead of relying on magics or heuristics. If you select a file reader and open a file, open it, and then click the reload-file button or go to View->Reload or press the ctrl-R keymap, the file is reloaded but using the magic/heuristics again instead of the file format reader you previously chose. Likewise, the Lua relaod() function has the same issue (which is how I found this problem). I have tested this change by hand, using a Lua script, but I didn't add it to the testsuite because I need another change for my test script to work correctly. (an enhancement rather than a bug fix, which I'll submit separately) Change-Id: I48c2d9ea443e37fd9d41be43d6b6cd5a866d5b01 Reviewed-on: https://code.wireshark.org/review/764 Reviewed-by: Anders Broman --- cfile.h | 1 + epan/wslua/wslua.h | 8 +++++--- epan/wslua/wslua_file.c | 12 ++++++------ file.c | 18 +++++++++++++++++- rawshark.c | 1 + reordercap.c | 2 ++ tfshark.c | 3 +++ tshark.c | 3 +++ ui/gtk/capture_file_dlg.c | 4 ++-- ui/gtk/funnel_stat.c | 2 +- ui/qt/main_window_slots.cpp | 2 +- ui/tap_export_pdu.c | 1 + 12 files changed, 43 insertions(+), 14 deletions(-) diff --git a/cfile.h b/cfile.h index f34c34ee14..40ae6057f0 100644 --- a/cfile.h +++ b/cfile.h @@ -72,6 +72,7 @@ typedef struct _capture_file { gboolean unsaved_changes; /* Does the capture file have changes that have not been saved? */ gint64 f_datalen; /* Size of capture file data (uncompressed) */ guint16 cd_t; /* File type of capture file */ + unsigned int open_type; /* open_routine index+1 used, if selected, or WTAP_TYPE_AUTO */ gboolean iscompressed; /* TRUE if the file is compressed */ int lnk_t; /* File link-layer type; could be WTAP_ENCAP_PER_PACKET */ GArray *linktypes; /* Array of packet link-layer types */ diff --git a/epan/wslua/wslua.h b/epan/wslua/wslua.h index 4bc16062dc..bdf473b143 100644 --- a/epan/wslua/wslua.h +++ b/epan/wslua/wslua.h @@ -573,15 +573,17 @@ extern int wslua_set__index(lua_State *L); return luaL_error(L, "%s's attribute `%s' must be a string or nil", #C , #field ); \ } \ if (obj->member != NULL && need_free) \ - free((void*) obj->member); \ + g_free((void*) obj->member); \ obj->member = s; \ return 0; \ - } + } \ + /* silly little trick so we can add a semicolon after this macro */ \ + static int C##_set_##field(lua_State*) #define WSLUA_ATTRIBUTE_STRING_SETTER(C,field,need_free) \ WSLUA_ATTRIBUTE_NAMED_STRING_SETTER(C,field,field,need_free) -#define WSLUA_ERROR(name,error) { luaL_error(L, ep_strdup_printf("%s%s", #name ": " ,error) ); } +#define WSLUA_ERROR(name,error) { luaL_error(L, "%s%s", #name ": " ,error); } #define WSLUA_ARG_ERROR(name,attr,error) { luaL_argerror(L,WSLUA_ARG_ ## name ## _ ## attr, #name ": " error); } #define WSLUA_OPTARG_ERROR(name,attr,error) { luaL_argerror(L,WSLUA_OPTARG_##name##_ ##attr, #name ": " error); } diff --git a/epan/wslua/wslua_file.c b/epan/wslua/wslua_file.c index e628b65083..9ac748c146 100644 --- a/epan/wslua/wslua_file.c +++ b/epan/wslua/wslua_file.c @@ -238,9 +238,9 @@ static int pushresult (lua_State *L, int i, const char *filename) { else { lua_pushnil(L); if (filename) - lua_pushfstring(L, "%s: %s", filename, strerror(en)); + lua_pushfstring(L, "%s: %s", filename, g_strerror(en)); else - lua_pushfstring(L, "%s", strerror(en)); + lua_pushfstring(L, "%s", g_strerror(en)); lua_pushinteger(L, en); return 3; } @@ -368,7 +368,7 @@ static int File_lines_iterator(lua_State* L) { success = File_read_line(L, ft); /* if (ferror(ft)) - return luaL_error(L, "%s", strerror(errno)); + return luaL_error(L, "%s", g_strerror(errno)); */ return success; } @@ -421,7 +421,7 @@ WSLUA_METHOD File_write(lua_State* L) { if (!status) { lua_pop(L,1); /* pop the extraneous File object */ lua_pushnil(L); - lua_pushfstring(L, "File write error: %s", strerror(err)); + lua_pushfstring(L, "File write error: %s", g_strerror(err)); lua_pushinteger(L, err); return 3; } @@ -1157,7 +1157,7 @@ WSLUA_METHOD FrameInfoConst_write_data(lua_State* L) { if (!wtap_dump_file_write(fh->wdh, fi->pd, (size_t)(len), &err)) { lua_pushboolean(L, FALSE); - lua_pushfstring(L, "FrameInfoConst write_data() error: %s", strerror(err)); + lua_pushfstring(L, "FrameInfoConst write_data() error: %s", g_strerror(err)); lua_pushnumber(L, err); return 3; } @@ -1464,7 +1464,7 @@ wslua_filehandler_read(wtap *wth, int *err, gchar **err_info, switch ( lua_pcall(L,2,1,1) ) { case 0: if (lua_isnumber(L,-1)) { - *data_offset = (gint64) lua_tonumber(L, -1); + *data_offset = wslua_togint64(L, -1); retval = 1; break; } diff --git a/file.c b/file.c index d57e0a9b8b..a37a0524ab 100644 --- a/file.c +++ b/file.c @@ -376,6 +376,7 @@ cf_open(capture_file *cf, const char *fname, unsigned int type, gboolean is_temp cf->computed_elapsed = 0; cf->cd_t = wtap_file_type_subtype(cf->wth); + cf->open_type = type; cf->linktypes = g_array_sized_new(FALSE, FALSE, (guint) sizeof(int), 1); cf->count = 0; cf->packet_comment_count = 0; @@ -470,6 +471,9 @@ cf_reset_state(capture_file *cf) /* ...which means we have no changes to that file to save. */ cf->unsaved_changes = FALSE; + /* no open_routine type */ + cf->open_type = WTAP_TYPE_AUTO; + /* Free up the packet buffer. */ buffer_free(&cf->buf); @@ -4334,6 +4338,11 @@ rescan_file(capture_file *cf, const char *fname, gboolean is_tempfile, int *err) wtap_close(cf->wth); /* Open the new file. */ + /* XXX: this will go through all open_routines for a matching one. But right + now rescan_file() is only used when a file is being saved to a different + format than the original, and the user is not given a choice of which + reader to use (only which format to save it in), so doing this makes + sense for now. */ cf->wth = wtap_open_offline(fname, WTAP_TYPE_AUTO, err, &err_info, TRUE); if (cf->wth == NULL) { cf_open_failure_alert_box(fname, *err, err_info, FALSE, 0); @@ -4768,6 +4777,10 @@ cf_save_packets(capture_file *cf, const char *fname, guint save_format, the wtap structure, the filename, and the "is temporary" status applies to the new file; just update that. */ wtap_close(cf->wth); + /* Although we're just "copying" and then opening the copy, it will + try all open_routine readers to open the copy, so we need to + reset the cfile's open_type. */ + cf->open_type = WTAP_TYPE_AUTO; cf->wth = wtap_open_offline(fname, WTAP_TYPE_AUTO, &err, &err_info, TRUE); if (cf->wth == NULL) { cf_open_failure_alert_box(fname, err, err_info, FALSE, 0); @@ -4799,6 +4812,9 @@ cf_save_packets(capture_file *cf, const char *fname, guint save_format, ...as long as, for gzipped files, the process of writing out the file *also* generates the information needed to support fast random access to the compressed file. */ + /* rescan_file will cause us to try all open_routines, so + reset cfile's open_type */ + cf->open_type = WTAP_TYPE_AUTO; if (rescan_file(cf, fname, FALSE, &err) != CF_READ_OK) { /* The rescan failed; just close the file. Either a dialog was popped up for the failure, so the @@ -5210,7 +5226,7 @@ cf_reload(capture_file *cf) { filename = g_strdup(cf->filename); is_tempfile = cf->is_tempfile; cf->is_tempfile = FALSE; - if (cf_open(cf, filename, WTAP_TYPE_AUTO, is_tempfile, &err) == CF_OK) { + if (cf_open(cf, filename, cf->open_type, is_tempfile, &err) == CF_OK) { switch (cf_read(cf, TRUE)) { case CF_READ_OK: diff --git a/rawshark.c b/rawshark.c index e7aed58c5f..f68941beda 100644 --- a/rawshark.c +++ b/rawshark.c @@ -1678,6 +1678,7 @@ raw_cf_open(capture_file *cf, const char *fname) cf->unsaved_changes = FALSE; cf->cd_t = WTAP_FILE_TYPE_SUBTYPE_UNKNOWN; + cf->open_type = WTAP_TYPE_AUTO; cf->count = 0; cf->drops_known = FALSE; cf->drops = 0; diff --git a/reordercap.c b/reordercap.c index 0c7e00a383..2587afe098 100644 --- a/reordercap.c +++ b/reordercap.c @@ -222,6 +222,8 @@ int main(int argc, char *argv[]) init_open_routines(); /* Open infile */ + /* TODO: if reordercap is ever changed to give the user a choice of which + open_routine reader to use, then the following needs to change. */ wth = wtap_open_offline(infile, WTAP_TYPE_AUTO, &err, &err_info, TRUE); if (wth == NULL) { fprintf(stderr, "reordercap: Can't open %s: %s\n", infile, diff --git a/tfshark.c b/tfshark.c index 0489d3796a..9d6197118b 100644 --- a/tfshark.c +++ b/tfshark.c @@ -1425,6 +1425,8 @@ main(int argc, char *argv[]) relinquish_special_privs_perm(); print_current_user(); + /* TODO: if tfshark is ever changed to give the user a choice of which + open_routine reader to use, then the following needs to change. */ if (cf_open(&cfile, cf_name, WTAP_TYPE_AUTO, FALSE, &err) != CF_OK) { epan_cleanup(); return 2; @@ -2543,6 +2545,7 @@ cf_open(capture_file *cf, const char *fname, unsigned int type, gboolean is_temp cf->unsaved_changes = FALSE; cf->cd_t = ftap_file_type_subtype((struct ftap*)cf->wth); /**** XXX - DOESN'T WORK RIGHT NOW!!!! */ + cf->open_type = type; cf->count = 0; cf->drops_known = FALSE; cf->drops = 0; diff --git a/tshark.c b/tshark.c index c5f9aa4c56..62208a0c6e 100644 --- a/tshark.c +++ b/tshark.c @@ -2626,6 +2626,8 @@ capture_input_new_file(capture_session *cap_session, gchar *new_file) /* if we are in real-time mode, open the new file now */ if (do_dissection) { + /* this is probably unecessary, but better safe than sorry */ + ((capture_file *)cap_session->cf)->open_type = WTAP_TYPE_AUTO; /* Attempt to open the capture file and set up to read from it. */ switch(cf_open((capture_file *)cap_session->cf, capture_opts->save_file, WTAP_TYPE_AUTO, is_tempfile, &err)) { case CF_OK: @@ -4009,6 +4011,7 @@ cf_open(capture_file *cf, const char *fname, unsigned int type, gboolean is_temp cf->unsaved_changes = FALSE; cf->cd_t = wtap_file_type_subtype(cf->wth); + cf->open_type = type; cf->count = 0; cf->drops_known = FALSE; cf->drops = 0; diff --git a/ui/gtk/capture_file_dlg.c b/ui/gtk/capture_file_dlg.c index e350342c0a..baef2baed9 100644 --- a/ui/gtk/capture_file_dlg.c +++ b/ui/gtk/capture_file_dlg.c @@ -709,7 +709,7 @@ file_open_cmd(capture_file *cf, GtkWidget *w _U_) continue; } - /* Try to open the capture file. */ + /* Try to open the capture file. This closes the current file if it succeeds. */ if (cf_open(&cfile, file_name->str, type, FALSE, &err) != CF_OK) { /* We couldn't open it; don't dismiss the open dialog box, just leave it around so that the user can, after they @@ -990,7 +990,7 @@ file_merge_cmd(GtkWidget *w _U_) cf_close(&cfile); - /* Try to open the merged capture file. */ + /* Try to open the merged capture file. This closes the current file if it succeeds. */ if (cf_open(&cfile, tmpname, WTAP_TYPE_AUTO, TRUE /* temporary file */, &err) != CF_OK) { /* We couldn't open it; fail. */ if (rfcode != NULL) diff --git a/ui/gtk/funnel_stat.c b/ui/gtk/funnel_stat.c index 1b3dc00662..5d03fefbde 100644 --- a/ui/gtk/funnel_stat.c +++ b/ui/gtk/funnel_stat.c @@ -515,7 +515,7 @@ static gboolean funnel_open_file(const char* fname, const char* filter, const ch } } - + /* This closes the current file if it succeeds. */ if (cf_open(&cfile, fname, WTAP_TYPE_AUTO, FALSE, &err) != CF_OK) { *err_str = g_strerror(err); if (rfcode != NULL) dfilter_free(rfcode); diff --git a/ui/qt/main_window_slots.cpp b/ui/qt/main_window_slots.cpp index ddc13e2d84..78a175d15b 100644 --- a/ui/qt/main_window_slots.cpp +++ b/ui/qt/main_window_slots.cpp @@ -150,7 +150,7 @@ void MainWindow::openCaptureFile(QString& cf_path, QString& display_filter, unsi } } - /* Try to open the capture file. */ + /* Try to open the capture file. This closes the current file if it succeeds. */ cfile.window = this; if (cf_open(&cfile, cf_path.toUtf8().constData(), type, FALSE, &err) != CF_OK) { /* We couldn't open it; don't dismiss the open dialog box, diff --git a/ui/tap_export_pdu.c b/ui/tap_export_pdu.c index 7c35ad36d5..2191c40066 100644 --- a/ui/tap_export_pdu.c +++ b/ui/tap_export_pdu.c @@ -161,6 +161,7 @@ exp_pdu_file_open(exp_pdu_t *exp_pdu_tap_data) remove_tap_listener(exp_pdu_tap_data); + /* XXX: should this use the open_routine type in the cfile instead of WTAP_TYPE_AUTO? */ if (cf_open(&cfile, capfile_name, WTAP_TYPE_AUTO, TRUE /* temporary file */, &err) != CF_OK) { open_failure_alert_box(capfile_name, err, FALSE); goto end;