wiretap: candump: Don't generate a temporary PCAP file

It's preferable to parse text files and generate packets on demand,
rather than generate a temporary PCAP file and dump all available
packets into it.

Parsing on the fly has a benefit of handling damaged files up to the
point of damage, while the approach with a temporary file doesn't
allow either to report that the original file is damaged or perform
conversion in the first place.

This version works faster than the previous one.

Command:
time ./run/tshark -r ./candump-2019-07-01_111120.log.gz > /dev/null

The test file is attached to the bug 15889

The current version:
real    0m0,597s
user    0m0,533s
sys     0m0,118s

The previous version:
real    0m2,176s
user    0m1,966s
sys     0m0,100s

Bug: 15889
Change-Id: I862ce47752531c2e9d9459f5d865c1fc08f32fea
Reviewed-on: https://code.wireshark.org/review/34007
Petri-Dish: Anders Broman <a.broman58@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>

4 files changed, 149 insertions, 248 deletions

diff --git a/wiretap/candump.c b/wiretap/candump.c
index 0fe484d322..987e304f18 100644
--- a/wiretap/candump.c
+++ b/wiretap/candump.c
@@ -13,10 +13,9 @@
 #include <wtap-int.h>
 #include <file_wrappers.h>
 #include <epan/exported_pdu.h>
-#include <wsutil/tempfile.h>
-#include <wsutil/os_version_info.h>
 #include <string.h>
 #include <inttypes.h>
+#include <errno.h>
 #include "candump.h"
 #include "candump_priv.h"
 
@@ -26,10 +25,9 @@ static gboolean candump_read(wtap *wth, wtap_rec *rec, Buffer *buf,
 static gboolean candump_seek_read(wtap *wth, gint64 seek_off,
                                   wtap_rec *rec, Buffer *buf,
                                   int *err, gchar **err_info);
-static void candump_close(wtap *wth);
 
-static gboolean
-candump_add_packet(wtap_dumper *wdh, const msg_t *msg, int *err, char **err_info)
+static void
+candump_write_packet(wtap_rec *rec, Buffer *buf, const msg_t *msg)
 {
     static const char *can_proto_name    = "can-hostendian";
     static const char *canfd_proto_name  = "canfd";
@@ -38,9 +36,7 @@ candump_add_packet(wtap_dumper *wdh, const msg_t *msg, int *err, char **err_info
     guint              header_length;
     guint              packet_length;
     guint              frame_length;
-
-    guint8    buf[128];
-    wtap_rec  rec;
+    guint8            *buf_data;
 
     /* Adjust proto name length to be aligned on 4 byte boundary */
     proto_name_length += (proto_name_length % 4) ? (4 - (proto_name_length % 4)) : 0;
@@ -49,10 +45,15 @@ candump_add_packet(wtap_dumper *wdh, const msg_t *msg, int *err, char **err_info
     frame_length  = msg->is_fd ? sizeof(canfd_frame_t) : sizeof(can_frame_t);
     packet_length = header_length + frame_length;
 
-    memset(buf, 0, sizeof(buf));
-    buf[1] = EXP_PDU_TAG_PROTO_NAME;
-    buf[3] = proto_name_length;
-    memcpy(buf + 4, proto_name, strlen(proto_name));
+    ws_buffer_clean(buf);
+    ws_buffer_assure_space(buf, packet_length);
+    buf_data = ws_buffer_start_ptr(buf);
+
+    memset(buf_data, 0, packet_length);
+
+    buf_data[1] = EXP_PDU_TAG_PROTO_NAME;
+    buf_data[3] = proto_name_length;
+    memcpy(buf_data + 4, proto_name, strlen(proto_name));
 
     if (msg->is_fd)
     {
@@ -64,7 +65,7 @@ candump_add_packet(wtap_dumper *wdh, const msg_t *msg, int *err, char **err_info
         canfd_frame.len    = msg->data.length;
         memcpy(canfd_frame.data, msg->data.data, msg->data.length);
 
-        memcpy(buf + header_length, (guint8 *)&canfd_frame, sizeof(canfd_frame));
+        memcpy(buf_data + header_length, (guint8 *)&canfd_frame, sizeof(canfd_frame));
     }
     else
     {
@@ -75,224 +76,102 @@ candump_add_packet(wtap_dumper *wdh, const msg_t *msg, int *err, char **err_info
         can_frame.can_dlc = msg->data.length;
         memcpy(can_frame.data, msg->data.data, msg->data.length);
 
-        memcpy(buf + header_length, (guint8 *)&can_frame, sizeof(can_frame));
+        memcpy(buf_data + header_length, (guint8 *)&can_frame, sizeof(can_frame));
     }
 
-    memset(&rec, 0, sizeof(rec));
-    rec.rec_type       = REC_TYPE_PACKET;
-    rec.presence_flags = WTAP_HAS_TS;
-    rec.ts             = msg->ts;
-    rec.tsprec         = WTAP_TSPREC_USEC;
-
-    rec.rec_header.packet_header.caplen = packet_length;
-    rec.rec_header.packet_header.len    = packet_length;
+    rec->rec_type       = REC_TYPE_PACKET;
+    rec->presence_flags = WTAP_HAS_TS;
+    rec->ts             = msg->ts;
+    rec->tsprec         = WTAP_TSPREC_USEC;
 
-    return wtap_dump(wdh, &rec, buf, err, err_info);
+    rec->rec_header.packet_header.caplen = packet_length;
+    rec->rec_header.packet_header.len    = packet_length;
 }
 
-static gchar *
-candump_dump(GSList *packets, int *err, char **err_info)
+static gboolean
+candump_parse(FILE_T fh, msg_t *msg, gint64 *offset, int *err, char **err_info)
 {
-    gchar        *filename;
-    int           import_file_fd;
-    wtap_dumper  *wdh;
-    GSList       *packet;
-
-    /* pcapng defs */
-    GArray                      *shb_hdrs = g_array_new(FALSE, FALSE, sizeof(wtap_block_t));
-    wtap_block_t                 shb_hdr;
-    wtapng_iface_descriptions_t *idb_inf  = NULL;
-    wtap_block_t                 int_data;
-    wtapng_if_descr_mandatory_t *int_data_mand;
-    GString                     *os_info_str;
-    gsize                        opt_len;
-    gchar                       *opt_str  = NULL;
-
-    static const gchar *opt_comment = "File converted to Exported PDU format during opening";
-    static const gchar *if_name     = "Fake IF";
+    candump_state_t state;
+    gboolean        ok;
+    gint64          seek_off;
 
 #ifdef CANDUMP_DEBUG
-    ws_debug_printf("%s: Creating a temporary file\n", G_STRFUNC);
+    ws_debug_printf("%s: Trying candump file decoder\n", G_STRFUNC);
 #endif
-    import_file_fd = create_tempfile(&filename, "Wireshark_PDU_candump_", NULL);
-
-    /* Now open a file and dump to it */
-    /* Create data for SHB */
-    os_info_str = g_string_new("");
-    get_os_version_info(os_info_str);
-
-    shb_hdr = wtap_block_create(WTAP_BLOCK_NG_SECTION);
-    /* options */
-    wtap_block_add_string_option(shb_hdr, OPT_COMMENT, opt_comment,
-                                 strlen(opt_comment));
-    /*
-     * UTF-8 string containing the name of the operating system used to create
-     * this section.
-     */
-    opt_len = os_info_str->len;
-    opt_str = g_string_free(os_info_str, FALSE);
-    if (opt_str)
-    {
-        wtap_block_add_string_option(shb_hdr, OPT_SHB_OS, opt_str, opt_len);
-        g_free(opt_str);
-    }
 
-    /*
-     * UTF-8 string containing the name of the application used to create
-     * this section. Avoid the precise version (get_appname_and_version) to
-     * avoid wiretap rebuilds when only the version changes.
-     */
-    wtap_block_add_string_option_format(shb_hdr, OPT_SHB_USERAPPL, "Wireshark %s", VERSION);
+    memset(&state, 0, sizeof(state));
+    state.fh = fh;
 
-    /* Add header to the array */
-    g_array_append_val(shb_hdrs, shb_hdr);
-
-    /* Create fake IDB info */
-    idb_inf = g_new(wtapng_iface_descriptions_t, 1);
-    idb_inf->interface_data = g_array_new(FALSE, FALSE, sizeof(wtap_block_t));
-
-    /* create the fake interface data */
-    int_data                             = wtap_block_create(WTAP_BLOCK_IF_DESCR);
-    int_data_mand                        = (wtapng_if_descr_mandatory_t *)wtap_block_get_mandatory_data(int_data);
-    int_data_mand->wtap_encap            = WTAP_ENCAP_WIRESHARK_UPPER_PDU;
-    int_data_mand->time_units_per_second = 1000000; /* default microsecond resolution */
-    int_data_mand->snap_len              = WTAP_MAX_PACKET_SIZE_STANDARD;
-
-    wtap_block_add_string_option(int_data, OPT_IDB_NAME, if_name, strlen(if_name));
-    int_data_mand->num_stat_entries     = 0; /* Number of ISB:s */
-    int_data_mand->interface_statistics = NULL;
-
-    g_array_append_val(idb_inf->interface_data, int_data);
-
-    const wtap_dump_params params = {
-        .encap    = WTAP_ENCAP_WIRESHARK_UPPER_PDU,
-        .snaplen  = WTAP_MAX_PACKET_SIZE_STANDARD,
-        .shb_hdrs = shb_hdrs,
-        .idb_inf  = idb_inf,
-    };
+    do
+    {
+        if (file_eof(fh))
+            return FALSE;
 
+        seek_off = file_tell(fh);
 #ifdef CANDUMP_DEBUG
-    ws_debug_printf("%s: Opening the temporary file for writing\n", G_STRFUNC);
+        ws_debug_printf("%s: Starting parser at offset %" PRIi64 "\n", G_STRFUNC, seek_off);
 #endif
-    wdh = wtap_dump_fdopen(import_file_fd, WTAP_FILE_TYPE_SUBTYPE_PCAPNG,
-                           WTAP_UNCOMPRESSED, &params, err);
+        state.file_bytes_read = 0;
+        ok = run_candump_parser(&state, err, err_info);
+
+        /* Rewind the file to the offset we have finished parsing */
+        if (file_seek(fh, seek_off + state.file_bytes_read, SEEK_SET, err) == -1)
+        {
+            g_free(*err_info);
+            *err      = errno;
+            *err_info = g_strdup(g_strerror(errno));
+            return FALSE;
+        }
+    }
+    while (ok && !state.is_msg_valid);
 
-    if (!wdh)
-        goto error_open;
+    if (!ok)
+        return FALSE;
 
 #ifdef CANDUMP_DEBUG
-    ws_debug_printf("%s: Writing packet data into the file\n", G_STRFUNC);
+    ws_debug_printf("%s: Success\n", G_STRFUNC);
 #endif
-    /* OK we've opened a new pcapng file and written the headers, time to do the packets */
-    for (packet = packets; packet; packet = g_slist_next(packet))
-    {
-        if (!candump_add_packet(wdh, (msg_t *)packet->data, err, err_info))
-            goto error_write;
-    }
 
-#ifdef CANDUMP_DEBUG
-    ws_debug_printf("%s: Closing the file\n", G_STRFUNC);
-#endif
-    /* Close the written file */
-    if (!wtap_dump_close(wdh, err))
-        goto error_write;
-
-    goto exit;
-
-error_write:
-    wtap_dump_close(wdh, err);
-    ws_unlink(filename);
-error_open:
-    g_free(filename);
-    filename = NULL;
-exit:
-    wtap_block_array_free(shb_hdrs);
-    wtap_free_idb_info(idb_inf);
-
-    return filename;
+    if (offset)
+        *offset = seek_off;
+
+    if (msg)
+        *msg = state.msg;
+
+    return TRUE;
 }
 
-static wtap_open_return_val
-candump_parse(candump_priv_t **priv, wtap *wth, int *err, char **err_info)
+wtap_open_return_val
+candump_open(wtap *wth, int *err, char **err_info)
 {
-    GSList *packets;
-    gchar  *filename;
-    wtap   *fh;
+    if (!candump_parse(wth->fh, NULL, NULL, err, err_info))
+    {
+        g_free(*err_info);
 
-#ifdef CANDUMP_DEBUG
-    ws_debug_printf("%s: Trying candump file decoder\n", G_STRFUNC);
-#endif
-    packets = run_candump_parser(wth->fh, err, err_info);
+        *err      = 0;
+        *err_info = NULL;
 
-    if (!packets)
         return WTAP_OPEN_NOT_MINE;
-
-    if (*err)
-    {
-        g_slist_free_full(packets, g_free);
-        return WTAP_OPEN_ERROR;
     }
 
 #ifdef CANDUMP_DEBUG
-    ws_debug_printf("%s: Creating a PCAPNG file with data we've just read\n", G_STRFUNC);
+    ws_debug_printf("%s: This is our file\n", G_STRFUNC);
 #endif
-    /* Dump packets into a temporary file */
-    filename = candump_dump(packets, err, err_info);
-    g_slist_free_full(packets, g_free);
 
-    if (!filename)
-        return WTAP_OPEN_ERROR;
-
-#ifdef CANDUMP_DEBUG
-    ws_debug_printf("%s: Opening the newly created file\n", G_STRFUNC);
-#endif
-    /* Now open the file for reading */
-    fh = wtap_open_offline(filename, WTAP_TYPE_AUTO,
-                           err, err_info,
-                           (wth->random_fh ? TRUE : FALSE));
-
-    if (!fh)
+    if (file_seek(wth->fh, 0, SEEK_SET, err) == -1)
     {
-        g_free(filename);
-        return WTAP_OPEN_ERROR;
-    }
-
-    *priv = g_new0(candump_priv_t, 1);
-
-    (*priv)->tmp_file     = fh;
-    (*priv)->tmp_filename = filename;
+        *err      = errno;
+        *err_info = g_strdup(g_strerror(errno));
 
-#ifdef CANDUMP_DEBUG
-    ws_debug_printf("%s: Ok\n", G_STRFUNC);
-#endif
-    return WTAP_OPEN_MINE;
-}
-
-wtap_open_return_val
-candump_open(wtap *wth, int *err, char **err_info)
-{
-    wtap_open_return_val  ret;
-    candump_priv_t       *priv = NULL;
-
-    ret = candump_parse(&priv, wth, err, err_info);
-
-    if (ret != WTAP_OPEN_MINE)
-        return ret;
-
-    if (!priv)
         return WTAP_OPEN_ERROR;
+    }
 
-    /* Copy header section block from the temp file */
-    wtap_block_copy(g_array_index(wth->shb_hdrs, wtap_block_t, 0), g_array_index(priv->tmp_file->shb_hdrs, wtap_block_t, 0));
-
-    wth->priv              = priv;
-    wth->file_type_subtype = WTAP_FILE_TYPE_SUBTYPE_PCAPNG;
-    wth->file_encap        = priv->tmp_file->file_encap;
-    wth->file_tsprec       = priv->tmp_file->file_tsprec;
+    wth->priv              = NULL;
+    wth->file_type_subtype = WTAP_FILE_TYPE_SUBTYPE_UNKNOWN;
+    wth->file_encap        = WTAP_ENCAP_WIRESHARK_UPPER_PDU;
+    wth->file_tsprec       = WTAP_TSPREC_USEC;
     wth->subtype_read      = candump_read;
     wth->subtype_seek_read = candump_seek_read;
-    wth->subtype_close     = candump_close;
-    wth->snapshot_length   = 0;
 
     return WTAP_OPEN_MINE;
 }
@@ -301,28 +180,48 @@ static gboolean
 candump_read(wtap *wth, wtap_rec *rec, Buffer *buf, int *err, gchar **err_info,
              gint64 *data_offset)
 {
-    candump_priv_t *priv = (candump_priv_t *)wth->priv;
+    msg_t msg;
 
-    return wtap_read(priv->tmp_file, rec, buf, err, err_info, data_offset);
+#ifdef CANDUMP_DEBUG
+    ws_debug_printf("%s: Try reading at offset %" PRIi64 "\n", G_STRFUNC, file_tell(wth->fh));
+#endif
 
+    if (!candump_parse(wth->fh, &msg, data_offset, err, err_info))
+        return FALSE;
+
+#ifdef CANDUMP_DEBUG
+    ws_debug_printf("%s: Stopped at offset %" PRIi64 "\n", G_STRFUNC, file_tell(wth->fh));
+#endif
+
+    candump_write_packet(rec, buf, &msg);
+
+    return TRUE;
 }
 
 static gboolean
 candump_seek_read(wtap *wth , gint64 seek_off, wtap_rec *rec,
                   Buffer *buf, int *err, gchar **err_info)
 {
-    candump_priv_t *priv = (candump_priv_t *)wth->priv;
+    msg_t msg;
 
-    return wtap_seek_read(priv->tmp_file, seek_off, rec, buf, err, err_info);
-}
+#ifdef CANDUMP_DEBUG
+    ws_debug_printf("%s: Read at offset %" PRIi64 "\n", G_STRFUNC, seek_off);
+#endif
 
-static void candump_close(wtap *wth)
-{
-    candump_priv_t *priv = (candump_priv_t *)wth->priv;
+    if (file_seek(wth->random_fh, seek_off, SEEK_SET, err) == -1)
+    {
+        *err      = errno;
+        *err_info = g_strdup(g_strerror(errno));
+
+        return FALSE;
+    }
+
+    if (!candump_parse(wth->random_fh, &msg, NULL, err, err_info))
+        return FALSE;
+
+    candump_write_packet(rec, buf, &msg);
 
-    wtap_close(priv->tmp_file);
-    ws_unlink(priv->tmp_filename);
-    g_free(priv->tmp_filename);
+    return TRUE;
 }
 
 /*
diff --git a/wiretap/candump_parser.lemon b/wiretap/candump_parser.lemon
index 705f6dc041..3f7781fb03 100644
--- a/wiretap/candump_parser.lemon
+++ b/wiretap/candump_parser.lemon
@@ -108,19 +108,16 @@ DIAG_OFF(unreachable-code)
 %type data48 { msg_data_t }
 %type data64 { msg_data_t }
 
-%start_symbol log
-
-log ::= lines .
-
-lines ::= line .
-lines ::= lines ENDL line .
+%start_symbol line
 
 line ::= maybe_spaces msg(M) .
 {
 #ifdef CANDUMP_DEBUG
     ws_debug_printf("%s: read message\n", G_STRFUNC);
 #endif
-    state->packets = g_slist_append(state->packets, g_memdup(&(M), sizeof(M)));
+
+    state->msg          = M;
+    state->is_msg_valid = TRUE;
 }
 
 line ::= maybe_spaces .
@@ -273,29 +270,23 @@ DIAG_ON(unreachable-code)
 #include "candump_scanner_lex.h"
 #include "candump_parser.h"
 
-GSList *
-run_candump_parser(FILE_T fh, int *err, gchar **err_info)
+gboolean
+run_candump_parser(candump_state_t *state, int *err, gchar **err_info)
 {
-    candump_state_t  state;
     int              lex_code;
     yyscan_t         scanner;
     void            *parser;
 
-    if (file_seek(fh, 0, SEEK_SET, err) == -1)
-        return NULL;
+    state->err         = 0;
+    state->err_info    = NULL;
+    state->parse_error = NULL;
 
-    *err      = 0;
-    *err_info = NULL;
-
-    memset(&state, 0, sizeof(state));
-    state.fh = fh;
-
-    if (candump_lex_init_extra(&state, &scanner) != 0)
+    if (candump_lex_init_extra(state, &scanner) != 0)
     {
         *err      = errno;
         *err_info = g_strdup(g_strerror(errno));
 
-        return NULL;
+        return FALSE;
     }
 
     parser = CandumpParserAlloc(g_malloc);
@@ -311,7 +302,7 @@ run_candump_parser(FILE_T fh, int *err, gchar **err_info)
         lex_code = candump_lex(scanner);
 
 #ifdef CANDUMP_DEBUG
-        if (lex_code && lex_code != TOKEN_ENDL)
+        if (lex_code)
             ws_debug_printf("%s: Feeding %s '%s'\n",
                             G_STRFUNC, yyTokenName[lex_code],
                             candump_get_text(scanner));
@@ -320,9 +311,9 @@ run_candump_parser(FILE_T fh, int *err, gchar **err_info)
                             G_STRFUNC, yyTokenName[lex_code]);
 #endif
 
-        CandumpParser(parser, lex_code, state.token, &state);
+        CandumpParser(parser, lex_code, state->token, state);
 
-        if (state.err || state.err_info || state.parse_error)
+        if (state->err || state->err_info || state->parse_error)
             break;
     }
     while (lex_code);
@@ -334,25 +325,27 @@ run_candump_parser(FILE_T fh, int *err, gchar **err_info)
     CandumpParserFree(parser, g_free);
     candump_lex_destroy(scanner);
 
-    if (state.err || state.err_info || state.parse_error)
+    if (state->err || state->err_info || state->parse_error)
     {
-        if (state.err_info)
+        if (state->err_info)
         {
-            *err_info = state.err_info;
-            g_free(state.parse_error);
+            *err_info = state->err_info;
+            g_free(state->parse_error);
         }
         else
         {
-            *err_info = state.parse_error;
+            *err_info = state->parse_error;
         }
 
-        if (state.err)
-            *err = state.err;
+        if (state->err)
+            *err = state->err;
         else
             *err = WTAP_ERR_BAD_FILE;
+
+        return FALSE;
     }
 
-    return state.packets;
+    return TRUE;
 }
 
 }
diff --git a/wiretap/candump_priv.h b/wiretap/candump_priv.h
index 9b10ec8220..5d3ad7b156 100644
--- a/wiretap/candump_priv.h
+++ b/wiretap/candump_priv.h
@@ -63,9 +63,12 @@ typedef struct {
 } candump_priv_t;
 
 typedef struct {
-    GSList *packets;
+    gboolean is_msg_valid;
+    msg_t    msg;
 
     FILE_T  fh;
+    guint64 file_bytes_read;
+
     int     err;
     gchar  *err_info;
     gchar  *parse_error;
@@ -73,8 +76,8 @@ typedef struct {
     token_t token;
 } candump_state_t;
 
-GSList *
-run_candump_parser(FILE_T fh, int *err, gchar **err_info);
+gboolean
+run_candump_parser(candump_state_t *state, int *err, gchar **err_info);
 
 #include <wsutil/ws_printf.h>
 
diff --git a/wiretap/candump_scanner.l b/wiretap/candump_scanner.l
index 98b98ee791..b74f14cc17 100644
--- a/wiretap/candump_scanner.l
+++ b/wiretap/candump_scanner.l
@@ -48,22 +48,28 @@
 #define YY_NO_UNISTD_H
 #endif
 
-static int candump_yyinput(void *buf, unsigned int max_size,
-                           candump_state_t *state)
+static int candump_yyinput(void *buf, candump_state_t *state)
 {
-    int result = file_read(buf, max_size, state->fh);
+    int c = file_getc(state->fh);
 
-    if (result == EOF)
+    if (c == EOF)
     {
         state->err = file_error(state->fh, &state->err_info);
-        result     = YY_NULL;
+        return YY_NULL;
     }
 
-    return result;
+    *(char *)buf = c;
+
+    return 1;
 }
 
 #define YY_INPUT(buf, result, max_size) \
-    do { (result) = candump_yyinput((buf), (max_size), yyextra); } while (0)
+    do { (result) = candump_yyinput((buf), yyextra); } while (0)
+
+/* Count bytes read. This is required in order to rewind the file
+ * to the beginning of the next packet, since flex reads more bytes
+ * before executing the action that does yyterminate(). */
+#define YY_USER_ACTION do { yyextra->file_bytes_read += yyleng; } while (0);
 
 /*
  * Sleazy hack to suppress compiler warnings in yy_fatal_error().
@@ -87,7 +93,7 @@ HEX [0-9A-Fa-f]
 %%
 
 [ \t]                   { return TOKEN_SPACE; };
-[\r\n]                  { return TOKEN_ENDL; }
+[\r\n][ \t\r\n]*        { yyterminate(); }
 
 \({INT}+\.{INT}+\)      {
                             yyextra->token.v0 = strtoul(yytext + 1, NULL, 10);