Clean up some things.

Rename ascend_seek() to ascend_find_next_packet(), to indicate what it
does; it doesn't seek to an arbitrary place, it tries to find the
starting offset of the next packet when reading sequentially.

Don't have it set the header type - that's the job of the parser.

Don't set the "next packet seek start" when doing random access I/O -
that field is only for sequential I/O, and we don't want random I/O
happening at the same time (which can happen in Wireshark) interfering.

Clean up comments.

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

1 files changed, 34 insertions, 25 deletions

diff --git a/wiretap/ascendtext.c b/wiretap/ascendtext.c
index 0b9aeaac84..bc2b699913 100644
--- a/wiretap/ascendtext.c
+++ b/wiretap/ascendtext.c
@@ -71,12 +71,12 @@ static gboolean ascend_seek_read(wtap *wth, gint64 seek_off,
 /* Seeks to the beginning of the next packet, and returns the
    byte offset at which the header for that packet begins.
    Returns -1 on failure. */
-static gint64 ascend_seek(wtap *wth, int *err, gchar **err_info)
+static gint64 ascend_find_next_packet(wtap *wth, int *err, gchar **err_info)
 {
   int byte;
   gint64 date_off = -1, cur_off, packet_off;
   size_t string_level[ASCEND_MAGIC_STRINGS];
-  guint string_i = 0, type = 0;
+  guint string_i = 0;
   static const gchar ascend_date[] = ASCEND_DATE;
   size_t ascend_date_len           = sizeof ascend_date - 1; /* strlen of a constant string */
   size_t ascend_date_string_level;
@@ -135,7 +135,6 @@ static gint64 ascend_seek(wtap *wth, int *err, gchar **err_info)
             packet_off = date_off;
           }
 
-          type = ascend_magic[string_i].type;
           goto found;
         }
       } else {
@@ -200,8 +199,6 @@ found:
   if (file_seek(wth->fh, packet_off, SEEK_SET, err) == -1)
     return -1;
 
-  wth->rec.rec_header.packet_header.pseudo_header.ascend.type = type;
-
   return packet_off;
 }
 
@@ -214,11 +211,11 @@ wtap_open_return_val ascend_open(wtap *wth, int *err, gchar **err_info)
   ascend_t *ascend;
 
   /* We haven't yet allocated a data structure for our private stuff;
-     set the pointer to null, so that "ascend_seek()" knows not to
-     fill it in. */
+     set the pointer to null, so that "ascend_find_next_packet()" knows
+     not to fill it in. */
   wth->priv = NULL;
 
-  offset = ascend_seek(wth, err, err_info);
+  offset = ascend_find_next_packet(wth, err, err_info);
   if (offset == -1) {
     if (*err != 0 && *err != WTAP_ERR_SHORT_READ)
       return WTAP_OPEN_ERROR;
@@ -269,9 +266,9 @@ wtap_open_return_val ascend_open(wtap *wth, int *err, gchar **err_info)
   ascend = (ascend_t *)g_malloc(sizeof(ascend_t));
   wth->priv = (void *)ascend;
 
-  /* The first packet we want to read is the one that "ascend_seek()"
-     just found; start searching for it at the offset at which it
-     found it. */
+  /* The first packet we want to read is the one that
+     "ascend_find_next_packet()" just found; start searching
+     for it at the offset at which it found it. */
   ascend->next_packet_seek_start = offset;
 
   /* MAXen and Pipelines report the time since reboot.  In order to keep
@@ -293,7 +290,8 @@ wtap_open_return_val ascend_open(wtap *wth, int *err, gchar **err_info)
    Returns TRUE if we got a packet, FALSE otherwise. */
 static gboolean
 parse_ascend(ascend_t *ascend, FILE_T fh, wtap_rec *rec, Buffer *buf,
-             guint length, int *err, gchar **err_info)
+             guint length, gint64 *next_packet_seek_start_ret,
+             int *err, gchar **err_info)
 {
   ascend_state_t parser_state;
   int retval;
@@ -302,19 +300,29 @@ parse_ascend(ascend_t *ascend, FILE_T fh, wtap_rec *rec, Buffer *buf,
   retval = run_ascend_parser(fh, rec, ws_buffer_start_ptr(buf), &parser_state,
                              err, err_info);
 
-  /* did we see any data (hex bytes)? if so, tip off ascend_seek()
-     as to where to look for the next packet, if any. If we didn't,
-     maybe this record was broken. Advance so we don't get into
-     an infinite loop reading a broken trace. */
+  /* Did we see any data (hex bytes)? */
   if (parser_state.first_hexbyte) {
-    ascend->next_packet_seek_start = parser_state.first_hexbyte;
+    /* Yes.  Provide the offset of the first byte so that our caller can
+       tip off ascend_find_next_packet() as to where to look for the next
+       packet, if any. */
+    if (next_packet_seek_start_ret != NULL)
+      *next_packet_seek_start_ret = parser_state.first_hexbyte;
   } else {
-    /* Sometimes, a header will be printed but the data will be omitted, or
-       worse -- two headers will be printed, followed by the data for each.
+    /* No.  Maybe this record was broken; sometimes, a header will be
+       printed but the data will be omitted, or worse -- two headers will
+       be printed, followed by the data for each.
+
        Because of this, we need to be fairly tolerant of what we accept
-       here.  If we didn't find any hex bytes, skip over what we've read so
-       far so we can try reading a new packet. */
-    ascend->next_packet_seek_start = file_tell(fh);
+       here.  Provide our current offset so that our caller can tell
+       ascend_find_next_packet() to skip over what we've read so far so
+       we can try reading a new packet.
+
+     . That keeps us from getting into an infinite loop reading a broken
+       trace. */
+    if (next_packet_seek_start_ret != NULL)
+      *next_packet_seek_start_ret = file_tell(fh);
+
+    /* Don't treat that as a fatal error; pretend the parse succeeded. */
     retval = 0;
   }
 
@@ -405,11 +413,12 @@ static gboolean ascend_read(wtap *wth, int *err, gchar **err_info,
                 SEEK_SET, err) == -1)
     return FALSE;
 
-  offset = ascend_seek(wth, err, err_info);
+  offset = ascend_find_next_packet(wth, err, err_info);
   if (offset == -1)
     return FALSE;
   if (!parse_ascend(ascend, wth->fh, &wth->rec, wth->rec_data,
-                   wth->snapshot_length, err, err_info))
+                   wth->snapshot_length, &ascend->next_packet_seek_start,
+                   err, err_info))
     return FALSE;
 
   /* Flex might have gotten an EOF and caused *err to be set to
@@ -434,7 +443,7 @@ static gboolean ascend_seek_read(wtap *wth, gint64 seek_off,
   if (file_seek(wth->random_fh, seek_off, SEEK_SET, err) == -1)
     return FALSE;
   if (!parse_ascend(ascend, wth->random_fh, rec, buf,
-                   wth->snapshot_length, err, err_info))
+                   wth->snapshot_length, NULL, err, err_info))
     return FALSE;
 
   /* Flex might have gotten an EOF and caused *err to be set to