wiretap: have wtap_dump_close() provide a "needs to be reloaded" indi…

…cation. 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.
dipsx · Mar 14, 2022 · 109b92b · 109b92b
1 parent 304dae3
commit 109b92b
Show file tree

Hide file tree

Showing 15 changed files with 42 additions and 35 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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,15 +5149,15 @@ 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.
              */
             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
@@ -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
@@ -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
@@ -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(&params);
         ret = OUTPUT_FILE_ERROR;

diff --git a/text2pcap.c b/text2pcap.c
@@ -1051,7 +1051,7 @@ main(int argc, char *argv[])
     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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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