From 109b92b5d796f2fb64491fe1976257bc976f1f40 Mon Sep 17 00:00:00 2001 From: Guy Harris Date: Mon, 14 Mar 2022 12:11:24 -0700 Subject: [PATCH] wiretap: have wtap_dump_close() provide a "needs to be reloaded" indication. This allows the "needs to be reloaded" indication to be set in the close process, as is the case for ERF; having a routine that returns the value of that indication is not useful if it gets seet in the close process, as the handle for the wtap_dumper is no longer valid after wtap_dump_close() finishes. We also get rid of wtap_dump_get_needs_reload(), as callers should get that information via the added argument to wtap_dump_close(). Fixes #17989. --- editcap.c | 8 ++++---- epan/dissectors/packet-snort.c | 2 +- epan/wslua/wslua_dumper.c | 4 ++-- extcap/etl.c | 4 ++-- file.c | 14 ++++++-------- packaging/debian/libwiretap0.symbols | 1 - randpkt_core/randpkt_core.c | 2 +- reordercap.c | 2 +- text2pcap.c | 2 +- tshark.c | 4 ++-- ui/qt/import_text_dialog.cpp | 2 +- ui/tap_export_pdu.c | 2 +- wiretap/file_access.c | 9 ++++----- wiretap/merge.c | 4 ++-- wiretap/wtap.h | 17 ++++++++++++++--- 15 files changed, 42 insertions(+), 35 deletions(-) diff --git a/editcap.c b/editcap.c index 555888ac37..9dfa26d8c7 100644 --- a/editcap.c +++ b/editcap.c @@ -1023,7 +1023,7 @@ editcap_dump_open(const char *filename, const wtap_dump_params *params, int close_err; gchar *close_err_info; - wtap_dump_close(pdh, &close_err, &close_err_info); + wtap_dump_close(pdh, NULL, &close_err, &close_err_info); g_free(close_err_info); wtap_block_unref(if_data_copy); return NULL; @@ -1808,7 +1808,7 @@ main(int argc, char *argv[]) } while (nstime_cmp(&rec->ts, &block_next) > 0) { /* time for the next file */ - if (!wtap_dump_close(pdh, &write_err, &write_err_info)) { + if (!wtap_dump_close(pdh, NULL, &write_err, &write_err_info)) { cfile_close_failure_message(filename, write_err, write_err_info); ret = WRITE_ERROR; @@ -1840,7 +1840,7 @@ main(int argc, char *argv[]) if (split_packet_count != 0) { /* time for the next file? */ if (written_count > 0 && (written_count % split_packet_count) == 0) { - if (!wtap_dump_close(pdh, &write_err, &write_err_info)) { + if (!wtap_dump_close(pdh, NULL, &write_err, &write_err_info)) { cfile_close_failure_message(filename, write_err, write_err_info); ret = WRITE_ERROR; @@ -2272,7 +2272,7 @@ main(int argc, char *argv[]) } } - if (!wtap_dump_close(pdh, &write_err, &write_err_info)) { + if (!wtap_dump_close(pdh, NULL, &write_err, &write_err_info)) { cfile_close_failure_message(filename, write_err, write_err_info); ret = WRITE_ERROR; goto clean_exit; diff --git a/epan/dissectors/packet-snort.c b/epan/dissectors/packet-snort.c index b8a0ee3fdb..4a90bf33ef 100644 --- a/epan/dissectors/packet-snort.c +++ b/epan/dissectors/packet-snort.c @@ -1400,7 +1400,7 @@ static void snort_cleanup(void) if (current_session.pdh) { int write_err; gchar *write_err_info; - if (!wtap_dump_close(current_session.pdh, &write_err, &write_err_info)) { + if (!wtap_dump_close(current_session.pdh, NULL, &write_err, &write_err_info)) { /* XXX - somehow report the error? */ g_free(write_err_info); } diff --git a/epan/wslua/wslua_dumper.c b/epan/wslua/wslua_dumper.c index a78b50dee1..31070daba7 100644 --- a/epan/wslua/wslua_dumper.c +++ b/epan/wslua/wslua_dumper.c @@ -302,7 +302,7 @@ WSLUA_METHOD Dumper_close(lua_State* L) { g_hash_table_remove(dumper_encaps,*dp); - if (!wtap_dump_close(*dp, &err, &err_info)) { + if (!wtap_dump_close(*dp, NULL, &err, &err_info)) { if (err_info != NULL) { luaL_error(L,"error closing: %s (%s)", wtap_strerror(err), err_info); @@ -585,7 +585,7 @@ static int Dumper__gc(lua_State* L) { g_hash_table_remove(dumper_encaps,*dp); - if (!wtap_dump_close(*dp, &err, &err_info)) { + if (!wtap_dump_close(*dp, NULL, &err, &err_info)) { if (err_info != NULL) { luaL_error(L,"error closing: %s (%s)", wtap_strerror(err), err_info); diff --git a/extcap/etl.c b/extcap/etl.c index 0186a13535..9baaf217cb 100644 --- a/extcap/etl.c +++ b/extcap/etl.c @@ -383,7 +383,7 @@ wtap_open_return_val etw_dump(const char* etl_filename, const char* pcapng_filen { if (*err == ERROR_SUCCESS) { - if (!wtap_dump_close(g_pdh, err, err_info)) + if (!wtap_dump_close(g_pdh, NULL, err, err_info)) { returnVal = WTAP_OPEN_ERROR; } @@ -392,7 +392,7 @@ wtap_open_return_val etw_dump(const char* etl_filename, const char* pcapng_filen { int err_ignore; gchar* err_info_ignore = NULL; - if (!wtap_dump_close(g_pdh, &err_ignore, &err_info_ignore)) + if (!wtap_dump_close(g_pdh, NULL, &err_ignore, &err_info_ignore)) { returnVal = WTAP_OPEN_ERROR; g_free(err_info_ignore); diff --git a/file.c b/file.c index 91689cbaf8..7f688a5cd8 100644 --- a/file.c +++ b/file.c @@ -4876,7 +4876,7 @@ cf_save_records(capture_file *cf, const char *fname, guint save_format, If we're writing to a temporary file, remove it. XXX - should we do so even if we're not writing to a temporary file? */ - wtap_dump_close(pdh, &err, &err_info); + wtap_dump_close(pdh, NULL, &err, &err_info); if (fname_new != NULL) ws_unlink(fname_new); cf_callback_invoke(cf_cb_file_save_stopped, NULL); @@ -4887,13 +4887,11 @@ cf_save_records(capture_file *cf, const char *fname, guint save_format, If we're writing to a temporary file, remove it. */ if (fname_new != NULL) ws_unlink(fname_new); - wtap_dump_close(pdh, &err, &err_info); + wtap_dump_close(pdh, NULL, &err, &err_info); goto fail; } - needs_reload = wtap_dump_get_needs_reload(pdh); - - if (!wtap_dump_close(pdh, &err, &err_info)) { + if (!wtap_dump_close(pdh, &needs_reload, &err, &err_info)) { cfile_close_failure_alert_box(fname, err, err_info); goto fail; } @@ -5141,7 +5139,7 @@ cf_export_specified_packets(capture_file *cf, const char *fname, If we're writing to a temporary file, remove it. XXX - should we do so even if we're not writing to a temporary file? */ - wtap_dump_close(pdh, &err, &err_info); + wtap_dump_close(pdh, NULL, &err, &err_info); if (fname_new != NULL) { ws_unlink(fname_new); g_free(fname_new); @@ -5151,7 +5149,7 @@ cf_export_specified_packets(capture_file *cf, const char *fname, case PSP_FAILED: /* Error while saving. */ - wtap_dump_close(pdh, &err, &err_info); + wtap_dump_close(pdh, NULL, &err, &err_info); /* * We don't report any error from closing; the error that caused * process_specified_records() to fail has already been reported. @@ -5159,7 +5157,7 @@ cf_export_specified_packets(capture_file *cf, const char *fname, goto fail; } - if (!wtap_dump_close(pdh, &err, &err_info)) { + if (!wtap_dump_close(pdh, NULL, &err, &err_info)) { cfile_close_failure_alert_box(fname, err, err_info); goto fail; } diff --git a/packaging/debian/libwiretap0.symbols b/packaging/debian/libwiretap0.symbols index 218240f8ed..9223f92b13 100644 --- a/packaging/debian/libwiretap0.symbols +++ b/packaging/debian/libwiretap0.symbols @@ -102,7 +102,6 @@ libwiretap.so.0 libwiretap0 #MINVER# wtap_dump_file_type_subtype@Base 3.3.2 wtap_dump_file_write@Base 1.12.0~rc1 wtap_dump_flush@Base 1.9.1 - wtap_dump_get_needs_reload@Base 2.5.0 wtap_dump_open@Base 1.9.1 wtap_dump_open_stdout@Base 2.0.0 wtap_dump_open_tempfile@Base 2.0.0 diff --git a/randpkt_core/randpkt_core.c b/randpkt_core/randpkt_core.c index 03ddac0b49..a0db6fec1e 100644 --- a/randpkt_core/randpkt_core.c +++ b/randpkt_core/randpkt_core.c @@ -646,7 +646,7 @@ gboolean randpkt_example_close(randpkt_example* example) gchar *err_info; gboolean ok = TRUE; - if (!wtap_dump_close(example->dump, &err, &err_info)) { + if (!wtap_dump_close(example->dump, NULL, &err, &err_info)) { cfile_close_failure_message(example->filename, err, err_info); ok = FALSE; } diff --git a/reordercap.c b/reordercap.c index 4598e0e222..b219751795 100644 --- a/reordercap.c +++ b/reordercap.c @@ -357,7 +357,7 @@ main(int argc, char *argv[]) g_ptr_array_free(frames, TRUE); /* Close outfile */ - if (!wtap_dump_close(pdh, &err, &err_info)) { + if (!wtap_dump_close(pdh, NULL, &err, &err_info)) { cfile_close_failure_message(outfile, err, err_info); wtap_dump_params_cleanup(¶ms); ret = OUTPUT_FILE_ERROR; diff --git a/text2pcap.c b/text2pcap.c index 55152ec354..aeadbaf4ea 100644 --- a/text2pcap.c +++ b/text2pcap.c @@ -1051,7 +1051,7 @@ clean_exit: if (wdh) { int err; char *err_info; - if (!wtap_dump_close(wdh, &err, &err_info)) { + if (!wtap_dump_close(wdh, NULL, &err, &err_info)) { cfile_close_failure_message(output_filename, err, err_info); ret = 2; } diff --git a/tshark.c b/tshark.c index 5244f5670e..f23c4426a9 100644 --- a/tshark.c +++ b/tshark.c @@ -3895,14 +3895,14 @@ process_cap_file(capture_file *cf, char *save_file, int out_file_type, } } /* Now close the capture file. */ - if (!wtap_dump_close(pdh, &err, &err_info)) { + if (!wtap_dump_close(pdh, NULL, &err, &err_info)) { cfile_close_failure_message(save_file, err, err_info); status = PROCESS_FILE_ERROR; } } else { /* We got a write error; it was reported, so just close the dump file without bothering to check for further errors. */ - wtap_dump_close(pdh, &err, &err_info); + wtap_dump_close(pdh, NULL, &err, &err_info); g_free(err_info); status = PROCESS_FILE_ERROR; } diff --git a/ui/qt/import_text_dialog.cpp b/ui/qt/import_text_dialog.cpp index 9cc2654ab2..dff793e2c9 100644 --- a/ui/qt/import_text_dialog.cpp +++ b/ui/qt/import_text_dialog.cpp @@ -530,7 +530,7 @@ int ImportTextDialog::exec() { } cleanup: /* free in reverse order of allocation */ - if (!wtap_dump_close(import_info_.wdh, &err, &err_info)) + if (!wtap_dump_close(import_info_.wdh, NULL, &err, &err_info)) { cfile_close_failure_alert_box(capfile_name_.toUtf8().constData(), err, err_info); } diff --git a/ui/tap_export_pdu.c b/ui/tap_export_pdu.c index 5c1030762c..6977020289 100644 --- a/ui/tap_export_pdu.c +++ b/ui/tap_export_pdu.c @@ -185,7 +185,7 @@ exp_pdu_close(exp_pdu_t *exp_pdu_tap_data, int *err, gchar **err_info) { gboolean status; - status = wtap_dump_close(exp_pdu_tap_data->wdh, err, err_info); + status = wtap_dump_close(exp_pdu_tap_data->wdh, NULL, err, err_info); wtap_block_array_free(exp_pdu_tap_data->shb_hdrs); wtap_free_idb_info(exp_pdu_tap_data->idb_inf); diff --git a/wiretap/file_access.c b/wiretap/file_access.c index adf98b88cd..d2911bc601 100644 --- a/wiretap/file_access.c +++ b/wiretap/file_access.c @@ -2621,7 +2621,8 @@ wtap_dump_flush(wtap_dumper *wdh, int *err) } gboolean -wtap_dump_close(wtap_dumper *wdh, int *err, gchar **err_info) +wtap_dump_close(wtap_dumper *wdh, gboolean *needs_reload, + int *err, gchar **err_info) { gboolean ret = TRUE; @@ -2643,6 +2644,8 @@ wtap_dump_close(wtap_dumper *wdh, int *err, gchar **err_info) } ret = FALSE; } + if (needs_reload != NULL) + *needs_reload = wdh->needs_reload; g_free(wdh->priv); wtap_block_array_free(wdh->interface_data); wtap_block_array_free(wdh->dsbs_initial); @@ -2709,10 +2712,6 @@ wtap_dump_discard_decryption_secrets(wtap_dumper *wdh) } } -gboolean wtap_dump_get_needs_reload(wtap_dumper *wdh) { - return wdh->needs_reload; -} - /* internally open a file for writing (compressed or not) */ #ifdef HAVE_ZLIB static WFILE_T diff --git a/wiretap/merge.c b/wiretap/merge.c index da4e20b585..05a48ad90a 100644 --- a/wiretap/merge.c +++ b/wiretap/merge.c @@ -952,7 +952,7 @@ merge_process_packets(wtap_dumper *pdh, const int file_type, cb->callback_func(MERGE_EVENT_DONE, count, in_files, in_file_count, cb->data); if (status == MERGE_OK || status == MERGE_USER_ABORTED) { - if (!wtap_dump_close(pdh, err, err_info)) + if (!wtap_dump_close(pdh, NULL, err, err_info)) status = MERGE_ERR_CANT_CLOSE_OUTFILE; } else { /* @@ -963,7 +963,7 @@ merge_process_packets(wtap_dumper *pdh, const int file_type, */ int close_err = 0; gchar *close_err_info = NULL; - (void)wtap_dump_close(pdh, &close_err, &close_err_info); + (void)wtap_dump_close(pdh, NULL, &close_err, &close_err_info); g_free(close_err_info); } diff --git a/wiretap/wtap.h b/wiretap/wtap.h index b0505b3a22..f10cf077f5 100644 --- a/wiretap/wtap.h +++ b/wiretap/wtap.h @@ -2121,16 +2121,27 @@ gboolean wtap_addrinfo_list_empty(addrinfo_lists_t *addrinfo_lists); WS_DLL_PUBLIC gboolean wtap_dump_set_addrinfo_list(wtap_dumper *wdh, addrinfo_lists_t *addrinfo_lists); WS_DLL_PUBLIC -gboolean wtap_dump_get_needs_reload(wtap_dumper *wdh); -WS_DLL_PUBLIC void wtap_dump_discard_decryption_secrets(wtap_dumper *wdh); /** * Closes open file handles and frees memory associated with wdh. Note that * shb_hdr, idb_inf and nrb_hdr are not freed by this routine. + * + * @param wdh handle for the file we're closing. + * @param[out] needs_reload if not null, points to a gboolean that will + * be set to TRUE if a full reload of the file would be required if + * this was done as part of a "Save" or "Save As" operation, FALSE + * if no full reload would be required. + * @param[out] err points to an int that will be set to an error code + * on failure. + * @param[out] err_info for some errors, points to a gchar * that will + * be set to a string giving more details of the error. + * + * @return TRUE on success, FALSE on failure. */ WS_DLL_PUBLIC -gboolean wtap_dump_close(wtap_dumper *wdh, int *err, gchar **err_info); +gboolean wtap_dump_close(wtap_dumper *wdh, gboolean *needs_reload, + int *err, gchar **err_info); /** * Return TRUE if we can write a file out with the given GArray of file