Take the error message generation out of the merge_files routines.

Have them just return the information needed for the caller to produce
an error message, and have the callers use the new cfile_ routines for
reporting errors.

This requires that the "write failure alert box" routine take the
*input* file name as an argument, so that, on a merge, if the problem is
that a record from a given input file can't be written out to the type
of output file we're generating, the input file name can be given, along
with the record number in that file.

Change-Id: If5a5e00539e7e652008a523dec92c0b359a48e71
Reviewed-on: https://code.wireshark.org/review/21257
Reviewed-by: Guy Harris <guy@alum.mit.edu>

2 files changed, 63 insertions, 215 deletions

diff --git a/wiretap/merge.c b/wiretap/merge.c
index 8ad6530c60..8605b7fbeb 100644
--- a/wiretap/merge.c
+++ b/wiretap/merge.c
@@ -811,166 +811,13 @@ map_phdr_interface_id(struct wtap_pkthdr *phdr, const merge_in_file_t *in_file)
     return TRUE;
 }
 
-static gchar*
-get_read_error_string(const merge_in_file_t *in_files, const guint in_file_count,
-                      const int *err, gchar **err_info)
-{
-    GString *err_message = g_string_new("");
-    gchar   *display_basename = NULL;
-    guint    i;
-
-    g_assert(in_files != NULL);
-    g_assert(err != NULL);
-    g_assert(err_info != NULL);
-
-    if (*err_info == NULL) {
-        *err_info = g_strdup("no information supplied");
-    }
-
-    /*
-     * Find the file on which we got the error, and report the error.
-     */
-    for (i = 0; i < in_file_count; i++) {
-        if (in_files[i].state == GOT_ERROR) {
-            display_basename = g_filename_display_basename(in_files[i].filename);
-
-            switch (*err) {
-
-                case WTAP_ERR_SHORT_READ:
-                    g_string_printf(err_message,
-                         "The capture file %s appears to have been cut short"
-                          " in the middle of a packet.", display_basename);
-                    break;
-
-                case WTAP_ERR_BAD_FILE:
-                    g_string_printf(err_message,
-                         "The capture file %s appears to be damaged or corrupt.\n(%s)",
-                         display_basename, *err_info);
-                    break;
-
-                case WTAP_ERR_DECOMPRESS:
-                    g_string_printf(err_message,
-                         "The compressed capture file %s appears to be damaged or corrupt.\n"
-                         "(%s)", display_basename, *err_info);
-                    break;
-
-                default:
-                    g_string_printf(err_message,
-                         "An error occurred while reading the"
-                         " capture file %s: %s.",
-                         display_basename,  wtap_strerror(*err));
-                    break;
-            }
-
-            g_free(display_basename);
-            break;
-        }
-    }
-
-    g_free(*err_info);
-    *err_info = g_string_free(err_message, FALSE);
-
-    return *err_info;
-}
-
-static gchar*
-get_write_error_string(const merge_in_file_t *in_file, const int file_type,
-                       const gchar* out_filename, const int *err, gchar **err_info)
-{
-    GString *err_message = g_string_new("");
-    gchar *display_basename = NULL;
-    int write_err;
-
-    /* in_file may be NULL */
-    g_assert(err != NULL);
-    g_assert(err_info != NULL);
-
-    if (*err_info == NULL) {
-        *err_info = g_strdup("no information supplied");
-    }
-
-    write_err = *err;
-
-    display_basename = g_filename_display_basename(in_file ? in_file->filename : "UNKNOWN");
-
-    if (write_err < 0) {
-
-        switch (write_err) {
-
-            case WTAP_ERR_UNWRITABLE_ENCAP:
-                /*
-                 * This is a problem with the particular frame we're writing and
-                 * the file type and subtype we're wwriting; note that, and
-                 * report the frame number and file type/subtype.
-                 */
-                g_string_printf(err_message,
-                    "Frame %u of \"%s\" has a network type that can't be saved in a \"%s\" file.\n",
-                    in_file ? in_file->packet_num : 0, display_basename,
-                    wtap_file_type_subtype_string(file_type));
-                break;
-
-            case WTAP_ERR_PACKET_TOO_LARGE:
-                /*
-                 * This is a problem with the particular frame we're writing and
-                 * the file type and subtype we're writing; note that, and report
-                 * the frame number and file type/subtype.
-                 */
-                g_string_printf(err_message,
-                    "Frame %u of \"%s\" is too large for a \"%s\" file.",
-                    in_file ? in_file->packet_num : 0, display_basename,
-                    wtap_file_type_subtype_string(file_type));
-                break;
-
-            case WTAP_ERR_UNWRITABLE_REC_TYPE:
-                /*
-                 * This is a problem with the particular record we're writing and
-                 * the file type and subtype we're writing; note that, and report
-                 * the record number and file type/subtype.
-                 */
-                g_string_printf(err_message,
-                    "Record %u of \"%s\" has a record type that can't be saved in a \"%s\" file.",
-                    in_file ? in_file->packet_num : 0, display_basename,
-                    wtap_file_type_subtype_string(file_type));
-                break;
-
-            case WTAP_ERR_UNWRITABLE_REC_DATA:
-                /*
-                 * This is a problem with the particular record we're writing and
-                 * the file type and subtype we're writing; note that, and report
-                 * the frame number and file type/subtype.
-                 */
-                g_string_printf(err_message,
-                    "Record %u of \"%s\" has data that can't be saved in a \"%s\" file.\n(%s)",
-                    in_file ? in_file->packet_num : 0, display_basename,
-                    wtap_file_type_subtype_string(file_type), *err_info);
-                break;
-
-            default:
-                g_string_printf(err_message,
-                    "An error occurred while writing to the file \"%s\": %s.",
-                    out_filename, wtap_strerror(write_err));
-                break;
-        }
-    }
-    else {
-        /* OS error. */
-        g_string_printf(err_message, file_write_error_message(write_err), out_filename);
-    }
-
-    g_free(display_basename);
-    g_free(*err_info);
-    *err_info = g_string_free(err_message, FALSE);
-
-    return *err_info;
-}
-
 static merge_result
-merge_process_packets(const gchar* out_filename, wtap_dumper *pdh,
-                      const int file_type,
+merge_process_packets(wtap_dumper *pdh, const int file_type,
                       merge_in_file_t *in_files, const guint in_file_count,
                       const gboolean do_append, guint snaplen,
                       merge_progress_callback_t* cb,
-                      int *err, gchar **err_info)
+                      int *err, gchar **err_info, guint *err_fileno,
+                      guint32 *err_framenum)
 {
     merge_result        status = MERGE_OK;
     merge_in_file_t    *in_file;
@@ -1066,37 +913,12 @@ merge_process_packets(const gchar* out_filename, wtap_dumper *pdh,
         (void)wtap_dump_close(pdh, &close_err);
     }
 
-    if (status != MERGE_OK) {
-        GString *err_message = NULL;
-        gchar   *display_basename = NULL;
-
-        switch(status) {
-
-            case MERGE_ERR_CANT_READ_INFILE:
-                *err_info = get_read_error_string(in_files, in_file_count, err, err_info);
-                break;
-
-            case MERGE_ERR_CANT_WRITE_OUTFILE: /* fall through */
-            case MERGE_ERR_CANT_CLOSE_OUTFILE:
-                *err_info = get_write_error_string(in_file, file_type, out_filename, err, err_info);
-                break;
-
-            case MERGE_ERR_BAD_PHDR_INTERFACE_ID:
-                display_basename = g_filename_display_basename(in_file ? in_file->filename : "UNKNOWN");
-                if (*err_info != NULL)
-                    g_free(*err_info);
-                err_message = g_string_new("");
-                g_string_printf(err_message,
-                    "Record %u of \"%s\" has an interface ID which does not match any IDB in its file.",
-                    in_file ? in_file->packet_num : 0, display_basename);
-                g_free(display_basename);
-                *err_info = g_string_free(err_message, FALSE);
-                break;
-
-            case MERGE_USER_ABORTED: /* not really an error */
-            default:
-                break;
-        }
+    if (status == MERGE_OK || in_file == NULL) {
+        *err_fileno = 0;
+        *err_framenum = 0;
+    } else {
+        *err_fileno = (guint)(in_file - in_files);
+        *err_framenum = in_file ? in_file->packet_num : 0;
     }
 
     return status;
@@ -1105,16 +927,15 @@ merge_process_packets(const gchar* out_filename, wtap_dumper *pdh,
 /*
  * Merges the files to an output file whose name is supplied as an argument,
  * based on given input, and invokes callback during execution. Returns
- * MERGE_OK on success, or a MERGE_ERR_XXX on failure; note that the passed-in
- * 'err' variable will be more specific to what failed, and err_info will
- * have pretty output.
+ * MERGE_OK on success, or a MERGE_ERR_XXX on failure.
  */
 merge_result
 merge_files(const gchar* out_filename, const int file_type,
             const char *const *in_filenames, const guint in_file_count,
             const gboolean do_append, const idb_merge_mode mode,
             guint snaplen, const gchar *app_name, merge_progress_callback_t* cb,
-            int *err, gchar **err_info, guint *err_fileno)
+            int *err, gchar **err_info, guint *err_fileno,
+            guint32 *err_framenum)
 {
     merge_in_file_t    *in_files = NULL;
     int                 frame_type = WTAP_ENCAP_PER_PACKET;
@@ -1129,6 +950,7 @@ merge_files(const gchar* out_filename, const int file_type,
     g_assert(err != NULL);
     g_assert(err_info != NULL);
     g_assert(err_fileno != NULL);
+    g_assert(err_framenum != NULL);
 
     /* if a callback was given, it has to have a callback function ptr */
     g_assert((cb != NULL) ? (cb->callback_func != NULL) : TRUE);
@@ -1139,6 +961,7 @@ merge_files(const gchar* out_filename, const int file_type,
     if (!merge_open_in_files(in_file_count, in_filenames, &in_files, cb,
                              err, err_info, err_fileno)) {
         merge_debug("merge_files: merge_open_in_files() failed with err=%d", *err);
+        *err_framenum = 0;
         return MERGE_ERR_CANT_OPEN_INFILE;
     }
 
@@ -1182,15 +1005,16 @@ merge_files(const gchar* out_filename, const int file_type,
         g_free(in_files);
         wtap_block_array_free(shb_hdrs);
         wtap_free_idb_info(idb_inf);
+        *err_framenum = 0;
         return MERGE_ERR_CANT_OPEN_OUTFILE;
     }
 
     if (cb)
         cb->callback_func(MERGE_EVENT_READY_TO_MERGE, 0, in_files, in_file_count, cb->data);
 
-    status = merge_process_packets(out_filename, pdh, file_type, in_files,
-                                   in_file_count, do_append, snaplen, cb,
-                                   err, err_info);
+    status = merge_process_packets(pdh, file_type, in_files, in_file_count,
+                                   do_append, snaplen, cb, err, err_info,
+                                   err_fileno, err_framenum);
 
     g_free(in_files);
     wtap_block_array_free(shb_hdrs);
@@ -1202,8 +1026,7 @@ merge_files(const gchar* out_filename, const int file_type,
 /*
  * Merges the files to a temporary file based on given input, and invokes
  * callback during execution. Returns MERGE_OK on success, or a MERGE_ERR_XXX
- * on failure; note that the passed-in 'err' variable will be more specific
- * to what failed, and err_info will have pretty output.
+ * on failure.
  */
 merge_result
 merge_files_to_tempfile(gchar **out_filenamep, const char *pfx,
@@ -1211,7 +1034,8 @@ merge_files_to_tempfile(gchar **out_filenamep, const char *pfx,
                         const guint in_file_count, const gboolean do_append,
                         const idb_merge_mode mode, guint snaplen,
                         const gchar *app_name, merge_progress_callback_t* cb,
-                        int *err, gchar **err_info, guint *err_fileno)
+                        int *err, gchar **err_info, guint *err_fileno,
+                        guint32 *err_framenum)
 {
     merge_in_file_t    *in_files = NULL;
     int                 frame_type = WTAP_ENCAP_PER_PACKET;
@@ -1226,6 +1050,7 @@ merge_files_to_tempfile(gchar **out_filenamep, const char *pfx,
     g_assert(err != NULL);
     g_assert(err_info != NULL);
     g_assert(err_fileno != NULL);
+    g_assert(err_framenum != NULL);
 
     /* if a callback was given, it has to have a callback function ptr */
     g_assert((cb != NULL) ? (cb->callback_func != NULL) : TRUE);
@@ -1239,6 +1064,7 @@ merge_files_to_tempfile(gchar **out_filenamep, const char *pfx,
     if (!merge_open_in_files(in_file_count, in_filenames, &in_files, cb,
                              err, err_info, err_fileno)) {
         merge_debug("merge_files: merge_open_in_files() failed with err=%d", *err);
+        *err_framenum = 0;
         return MERGE_ERR_CANT_OPEN_INFILE;
     }
 
@@ -1283,15 +1109,16 @@ merge_files_to_tempfile(gchar **out_filenamep, const char *pfx,
         g_free(in_files);
         wtap_block_array_free(shb_hdrs);
         wtap_free_idb_info(idb_inf);
+        *err_framenum = 0;
         return MERGE_ERR_CANT_OPEN_OUTFILE;
     }
 
     if (cb)
         cb->callback_func(MERGE_EVENT_READY_TO_MERGE, 0, in_files, in_file_count, cb->data);
 
-    status = merge_process_packets(*out_filenamep, pdh, file_type, in_files,
-                                   in_file_count, do_append, snaplen, cb,
-                                   err, err_info);
+    status = merge_process_packets(pdh, file_type, in_files, in_file_count,
+                                   do_append, snaplen, cb, err, err_info,
+                                   err_fileno, err_framenum);
 
     g_free(in_files);
     wtap_block_array_free(shb_hdrs);
@@ -1303,15 +1130,15 @@ merge_files_to_tempfile(gchar **out_filenamep, const char *pfx,
 /*
  * Merges the files to the standard output based on given input, and invokes
  * callback during execution. Returns MERGE_OK on success, or a MERGE_ERR_XXX
- * on failure; note that the passed-in 'err' variable will be more specific
- * to what failed, and err_info will have pretty output.
+ * on failure.
  */
 merge_result
 merge_files_to_stdout(const int file_type, const char *const *in_filenames,
                       const guint in_file_count, const gboolean do_append,
                       const idb_merge_mode mode, guint snaplen,
                       const gchar *app_name, merge_progress_callback_t* cb,
-                      int *err, gchar **err_info, guint *err_fileno)
+                      int *err, gchar **err_info, guint *err_fileno,
+                      guint32 *err_framenum)
 {
     merge_in_file_t    *in_files = NULL;
     int                 frame_type = WTAP_ENCAP_PER_PACKET;
@@ -1325,6 +1152,7 @@ merge_files_to_stdout(const int file_type, const char *const *in_filenames,
     g_assert(err != NULL);
     g_assert(err_info != NULL);
     g_assert(err_fileno != NULL);
+    g_assert(err_framenum != NULL);
 
     /* if a callback was given, it has to have a callback function ptr */
     g_assert((cb != NULL) ? (cb->callback_func != NULL) : TRUE);
@@ -1335,6 +1163,7 @@ merge_files_to_stdout(const int file_type, const char *const *in_filenames,
     if (!merge_open_in_files(in_file_count, in_filenames, &in_files, cb,
                              err, err_info, err_fileno)) {
         merge_debug("merge_files: merge_open_in_files() failed with err=%d", *err);
+        *err_framenum = 0;
         return MERGE_ERR_CANT_OPEN_INFILE;
     }
 
@@ -1378,15 +1207,16 @@ merge_files_to_stdout(const int file_type, const char *const *in_filenames,
         g_free(in_files);
         wtap_block_array_free(shb_hdrs);
         wtap_free_idb_info(idb_inf);
+        *err_framenum = 0;
         return MERGE_ERR_CANT_OPEN_OUTFILE;
     }
 
     if (cb)
         cb->callback_func(MERGE_EVENT_READY_TO_MERGE, 0, in_files, in_file_count, cb->data);
 
-    status = merge_process_packets("standard output", pdh, file_type, in_files,
-                                   in_file_count, do_append, snaplen, cb,
-                                   err, err_info);
+    status = merge_process_packets(pdh, file_type, in_files, in_file_count,
+                                   do_append, snaplen, cb, err, err_info,
+                                   err_fileno, err_framenum);
 
     g_free(in_files);
     wtap_block_array_free(shb_hdrs);
diff --git a/wiretap/merge.h b/wiretap/merge.h
index 21d0861cd8..534fb751e8 100644
--- a/wiretap/merge.h
+++ b/wiretap/merge.h
@@ -135,8 +135,13 @@ typedef struct {
  * @param app_name The application name performing the merge, used in SHB info
  * @param cb The callback information to use during execution
  * @param[out] err Set to the internal WTAP_ERR_XXX error code if it failed
- * @param[out] err_info Set to a descriptive error string, which must be g_free'd
- * @param[out] err_fileno Set to the input file number which failed, if it failed
+ *   with MERGE_ERR_CANT_OPEN_INFILE, MERGE_ERR_CANT_OPEN_OUTFILE,
+ *   MERGE_ERR_CANT_READ_INFILE, MERGE_ERR_CANT_WRITE_OUTFILE, or
+ *   MERGE_ERR_CANT_CLOSE_OUTFILE
+ * @param[out] err_info Additional information for some WTAP_ERR_XXX codes
+ * @param[out] err_fileno Set to the input file number which failed, if it
+ *   failed
+ * @param[out] err_framenum Set to the input frame number if it failed
  * @return the frame type
  */
 WS_DLL_PUBLIC merge_result
@@ -144,7 +149,8 @@ merge_files(const gchar* out_filename, const int file_type,
             const char *const *in_filenames, const guint in_file_count,
             const gboolean do_append, const idb_merge_mode mode,
             guint snaplen, const gchar *app_name, merge_progress_callback_t* cb,
-            int *err, gchar **err_info, guint *err_fileno);
+            int *err, gchar **err_info, guint *err_fileno,
+            guint32 *err_framenum);
 
 /** Merge the given input files to a temporary file
  *
@@ -160,8 +166,13 @@ merge_files(const gchar* out_filename, const int file_type,
  * @param app_name The application name performing the merge, used in SHB info
  * @param cb The callback information to use during execution
  * @param[out] err Set to the internal WTAP_ERR_XXX error code if it failed
- * @param[out] err_info Set to a descriptive error string, which must be g_free'd
- * @param[out] err_fileno Set to the input file number which failed, if it failed
+ *   with MERGE_ERR_CANT_OPEN_INFILE, MERGE_ERR_CANT_OPEN_OUTFILE,
+ *   MERGE_ERR_CANT_READ_INFILE, MERGE_ERR_CANT_WRITE_OUTFILE, or
+ *   MERGE_ERR_CANT_CLOSE_OUTFILE
+ * @param[out] err_info Additional information for some WTAP_ERR_XXX codes
+ * @param[out] err_fileno Set to the input file number which failed, if it
+ *   failed
+ * @param[out] err_framenum Set to the input frame number if it failed
  * @return the frame type
  */
 WS_DLL_PUBLIC merge_result
@@ -170,7 +181,8 @@ merge_files_to_tempfile(gchar **out_filenamep, const char *pfx,
                         const guint in_file_count, const gboolean do_append,
                         const idb_merge_mode mode, guint snaplen,
                         const gchar *app_name, merge_progress_callback_t* cb,
-                        int *err, gchar **err_info, guint *err_fileno);
+                        int *err, gchar **err_info, guint *err_fileno,
+                        guint32 *err_framenum);
 
 /** Merge the given input files to the standard output
  *
@@ -183,8 +195,13 @@ merge_files_to_tempfile(gchar **out_filenamep, const char *pfx,
  * @param app_name The application name performing the merge, used in SHB info
  * @param cb The callback information to use during execution
  * @param[out] err Set to the internal WTAP_ERR_XXX error code if it failed
- * @param[out] err_info Set to a descriptive error string, which must be g_free'd
- * @param[out] err_fileno Set to the input file number which failed, if it failed
+ *   with MERGE_ERR_CANT_OPEN_INFILE, MERGE_ERR_CANT_OPEN_OUTFILE,
+ *   MERGE_ERR_CANT_READ_INFILE, MERGE_ERR_CANT_WRITE_OUTFILE, or
+ *   MERGE_ERR_CANT_CLOSE_OUTFILE
+ * @param[out] err_info Additional information for some WTAP_ERR_XXX codes
+ * @param[out] err_fileno Set to the input file number which failed, if it
+ *   failed
+ * @param[out] err_framenum Set to the input frame number if it failed
  * @return the frame type
  */
 WS_DLL_PUBLIC merge_result
@@ -192,7 +209,8 @@ merge_files_to_stdout(const int file_type, const char *const *in_filenames,
                       const guint in_file_count, const gboolean do_append,
                       const idb_merge_mode mode, guint snaplen,
                       const gchar *app_name, merge_progress_callback_t* cb,
-                      int *err, gchar **err_info, guint *err_fileno);
+                      int *err, gchar **err_info, guint *err_fileno,
+                      guint32 *err_framenum);
 
 #ifdef __cplusplus
 }