epan: Do not add dependent packets more than once

Do not add a dependent frame if it's already been added to a
frame's list. Do not mark a frame as a dependent of a displayed
frame if we've already marked it as such in this pass.

Clear the list of dependent frames if we reset the frame data,
because the list of dependent frames depends on the dissection
and may not be valid if redissecting (because, for example,
a reassembly preference may have changed.)

Move the pointer to the list of dependent frames away from the
bitfields to a location that minimizes the struct size.

Fixup f870c6085d
Fix #18809

epan/frame_data.c

@@ -298,6 +298,11 @@ frame_data_reset(frame_data *fdata)
     g_slist_free(fdata->pfd);
     fdata->pfd = NULL;
   }
+
+  if (fdata->dependent_frames) {
+    g_slist_free(fdata->dependent_frames);
+    fdata->dependent_frames = NULL;
+  }
 }
 
 void

epan/frame_data.h

@@ -71,13 +71,13 @@ typedef struct _frame_data {
      LLP64 (64-bit Windows) platforms.  Put them here, one after the
      other, so they don't require padding between them. */
   GSList      *pfd;          /**< Per frame proto data */
+  GSList      *dependent_frames;     /**< A list of frames which this one depends on */
   const struct _color_filter *color_filter;  /**< Per-packet matching color_filter_t object */
   guint16      subnum;       /**< subframe number, for protocols that require this */
   /* Keep the bitfields below to 16 bits, so this plus the previous field
      are 32 bits. */
   unsigned int passed_dfilter   : 1; /**< 1 = display, 0 = no display */
   unsigned int dependent_of_displayed : 1; /**< 1 if a displayed frame depends on this frame */
-  GSList      *dependent_frames;     /**< A list of frames which this one depends on */
   /* Do NOT use packet_char_enc enum here: MSVC compiler does not handle an enum in a bit field properly */
   unsigned int encoding         : 1; /**< Character encoding (ASCII, EBCDIC...) */
   unsigned int visited          : 1; /**< Has this packet been visited yet? 1=Yes,0=No*/

epan/frame_data_sequence.c

@@ -320,8 +320,13 @@ find_and_mark_frame_depended_upon(gpointer data, gpointer user_data)
 
   if (dependent_frame && frames) {
     dependent_fd = frame_data_sequence_find(frames, dependent_frame);
-    dependent_fd->dependent_of_displayed = 1;
-    g_slist_foreach(dependent_fd->dependent_frames, find_and_mark_frame_depended_upon, frames);
+    /* Don't recurse for packets we've already marked. Note we assume that no
+     * packet depends on a future packet; we assume that in other places too.
+     */
+    if (!(dependent_fd->dependent_of_displayed || dependent_fd->passed_dfilter)) {
+      dependent_fd->dependent_of_displayed = 1;
+      g_slist_foreach(dependent_fd->dependent_frames, find_and_mark_frame_depended_upon, frames);
+    }
   }
 }
 

epan/packet.c

@@ -449,7 +449,16 @@ mark_frame_as_depended_upon(frame_data *fd, guint32 frame_num)
 {
 	/* Don't mark a frame as dependent on itself */
 	if (frame_num != fd->num) {
-		fd->dependent_frames = g_slist_prepend(fd->dependent_frames, GUINT_TO_POINTER(frame_num));
+		/* ws_assert(frame_num < fd->num) - we assume in several other
+		 * places in the code that frames don't depend on future
+		 * frames. */
+		/* XXX: Looking to see if the frame is already there is slow
+		 * if there's a lot of dependent frames, so this should
+		 * be a hash table or something.
+		 */
+		if (g_slist_find(fd->dependent_frames, GUINT_TO_POINTER(frame_num)) == NULL) {
+			fd->dependent_frames = g_slist_prepend(fd->dependent_frames, GUINT_TO_POINTER(frame_num));
+		}
 	}
 }