Wireshark · Ethereal-dev: Re: [ethereal-dev] wrong delta time values displayed

[Guy Harris <gharris@xxxxxxxxxxxx>]

On Sat, Sep 09, 2000 at 01:05:36PM +0200, Thomas Schaefer wrote:
> There is a problem with the computed delta-times between two following
> frames. It is independent if the absolute or relative time is displayed
> in summary. 

The delta times are computed from the absolute times - and, as the
relative times are just the differences between the absolute times and
the absolute time of the first packet, it wouldn't make a difference
whether they were computed from the absolute or relative times - so it's
not surprising that it's independent of which time is being displayed.

> Sometimes the delta-times (in displayed in summary or in frame details -
> it dosn't matter) have an negative sign. I think thats possible and not
> the root of the proplem. (timestamp and order of the frames may be
> different)

It is the root of the problem, in that Ethereal wasn't correctly
handling negative time differences.

Here's a patch to the code that's in the CVS tree at this instant, that
fixes it.  That patch may apply to the 0.8.11 version's source, as well,
although there's no guarantee of that; it almost certainly won't apply
to 0.8.3's source.

Download the 0.8.11 source, apply this patch, and, if the patch
succeeds (or it fails and you can fix the problems), rebuild. 
Otherwise, you'd have to get the current source from CVS, or grab one of
the nightly source snapshots (I don't know when they're made, so I don't
know if the 2000-09-10 snapshot will have it or if you'll have to wait
for the 2000-09-11 snapshot), and build that.

The change will also be in the next release.

Index: file.c
===================================================================
RCS file: /usr/local/cvsroot/ethereal/file.c,v
retrieving revision 1.216
diff -c -r1.216 file.c
*** file.c	2000/09/09 10:26:35	1.216
--- file.c	2000/09/10 06:35:33
***************
*** 614,628 ****
      firstusec = fdata->abs_usecs;
    }
  
-   /* Get the time elapsed between the first packet and this packet. */
-   cf->esec = fdata->abs_secs - firstsec;
-   if (firstusec <= fdata->abs_usecs) {
-     cf->eusec = fdata->abs_usecs - firstusec;
-   } else {
-     cf->eusec = (fdata->abs_usecs + 1000000) - firstusec;
-     cf->esec--;
-   }
- 
    fdata->cinfo = &cf->cinfo;
    for (i = 0; i < fdata->cinfo->num_cols; i++) {
      fdata->cinfo->col_data[i][0] = '\0';
--- 614,619 ----
***************
*** 692,709 ****
      }
  
      /* Get the time elapsed between the first packet and this packet. */
!     fdata->rel_secs = cf->esec;
!     fdata->rel_usecs = cf->eusec;
    
      /* Get the time elapsed between the previous displayed packet and
         this packet. */
!     fdata->del_secs = fdata->abs_secs - prevsec;
!     if (prevusec <= fdata->abs_usecs) {
!       fdata->del_usecs = fdata->abs_usecs - prevusec;
!     } else {
!       fdata->del_usecs = (fdata->abs_usecs + 1000000) - prevusec;
!       fdata->del_secs--;
!     }
      prevsec = fdata->abs_secs;
      prevusec = fdata->abs_usecs;
  
--- 683,704 ----
      }
  
      /* Get the time elapsed between the first packet and this packet. */
!     compute_timestamp_diff(&fdata->rel_secs, &fdata->rel_usecs,
! 		fdata->abs_secs, fdata->abs_usecs, firstsec, firstusec);
! 
!     /* If it's greater than the current elapsed time, set the elapsed time
!        to it (we check for "greater than" so as not to be confused by
!        time moving backwards). */
!     if (cf->esec < fdata->rel_secs
! 	|| (cf->esec == fdata->rel_secs && cf->eusec < fdata->rel_usecs)) {
!       cf->esec = fdata->rel_secs;
!       cf->eusec = fdata->rel_usecs;
!     }
    
      /* Get the time elapsed between the previous displayed packet and
         this packet. */
!     compute_timestamp_diff(&fdata->del_secs, &fdata->del_usecs,
! 		fdata->abs_secs, fdata->abs_usecs, prevsec, prevusec);
      prevsec = fdata->abs_secs;
      prevusec = fdata->abs_usecs;
  
Index: packet.c
===================================================================
RCS file: /usr/local/cvsroot/ethereal/packet.c,v
retrieving revision 1.103
diff -c -r1.103 packet.c
*** packet.c	2000/08/21 12:53:10	1.103
--- packet.c	2000/09/10 06:35:37
***************
*** 107,112 ****
--- 107,114 ----
  
  static gint ett_frame = -1;
  
+ static void display_signed_time(gchar *, int, gint32, gint32);
+ 
  /* Protocol-specific data attched to a frame_data structure - protocol
     index and opaque pointer. */
  typedef struct _frame_proto_data {
***************
*** 368,378 ****
          return cur;
  }
  
  gchar *
  rel_time_to_str(struct timeval *rel_time)
  {
          static gchar *cur;
!         static char str[3][10+1+6+1];
  
          if (cur == &str[0][0]) {
                  cur = &str[1][0];
--- 370,382 ----
          return cur;
  }
  
+ #define	REL_TIME_LEN	(1+10+1+6+1)
+ 
  gchar *
  rel_time_to_str(struct timeval *rel_time)
  {
          static gchar *cur;
!         static char str[3][REL_TIME_LEN];
  
          if (cur == &str[0][0]) {
                  cur = &str[1][0];
***************
*** 381,393 ****
          } else {
                  cur = &str[0][0];
          }
- 
- 	sprintf(cur, "%ld.%06ld", (long)rel_time->tv_sec,
- 		(long)rel_time->tv_usec);
  
          return cur;
  }
  
  /*
   * Given a pointer into a data buffer, and to the end of the buffer,
   * find the end of the (putative) line at that position in the data
--- 385,414 ----
          } else {
                  cur = &str[0][0];
          }
  
+ 	display_signed_time(cur, REL_TIME_LEN, rel_time->tv_sec,
+ 	    rel_time->tv_usec);
          return cur;
  }
  
+ static void
+ display_signed_time(gchar *buf, int buflen, gint32 sec, gint32 usec)
+ {
+ 	char *sign;
+ 
+ 	/* If the microseconds part of the time stamp is negative,
+ 	   print its absolute value and, if the seconds part isn't
+ 	   (the seconds part should be zero in that case), stick
+ 	   a "-" in front of the entire time stamp. */
+ 	sign = "";
+ 	if (usec < 0) {
+ 		usec = -usec;
+ 		if (sec >= 0)
+ 			sign = "-";
+ 	}
+ 	snprintf(buf, buflen, "%s%d.%06d", sign, sec, usec);
+ }
+ 
  /*
   * Given a pointer into a data buffer, and to the end of the buffer,
   * find the end of the (putative) line at that position in the data
***************
*** 820,834 ****
  static void
  col_set_rel_time(frame_data *fd, int col)
  {
!   snprintf(fd->cinfo->col_data[col], COL_MAX_LEN, "%d.%06d", fd->rel_secs,
!     fd->rel_usecs);
  }
  
  static void
  col_set_delta_time(frame_data *fd, int col)
  {
!   snprintf(fd->cinfo->col_data[col], COL_MAX_LEN, "%d.%06d", fd->del_secs,
!     fd->del_usecs);
  }
  
  /* Add "command-line-specified" time.
--- 841,855 ----
  static void
  col_set_rel_time(frame_data *fd, int col)
  {
!   display_signed_time(fd->cinfo->col_data[col], COL_MAX_LEN,
! 	fd->rel_secs, fd->rel_usecs);
  }
  
  static void
  col_set_delta_time(frame_data *fd, int col)
  {
!   display_signed_time(fd->cinfo->col_data[col], COL_MAX_LEN,
! 	fd->del_secs, fd->del_usecs);
  }
  
  /* Add "command-line-specified" time.
Index: packet.h
===================================================================
RCS file: /usr/local/cvsroot/ethereal/packet.h,v
retrieving revision 1.199
diff -c -r1.199 packet.h
*** packet.h	2000/08/30 02:50:01	1.199
--- packet.h	2000/09/10 06:35:38
***************
*** 98,109 ****
    guint32      num;       /* Frame number */
    guint32      pkt_len;   /* Packet length */
    guint32      cap_len;   /* Amount actually captured */
!   guint32      rel_secs;  /* Relative seconds */
!   guint32      rel_usecs; /* Relative microseconds */
    guint32      abs_secs;  /* Absolute seconds */
    guint32      abs_usecs; /* Absolute microseconds */
!   guint32      del_secs;  /* Delta seconds */
!   guint32      del_usecs; /* Delta microseconds */
    long         file_off;  /* File offset */
    column_info *cinfo;     /* Column formatting information */
    int          lnk_t;     /* Per-packet encapsulation/data-link type */
--- 98,109 ----
    guint32      num;       /* Frame number */
    guint32      pkt_len;   /* Packet length */
    guint32      cap_len;   /* Amount actually captured */
!   gint32       rel_secs;  /* Relative seconds (yes, it can be negative) */
!   gint32       rel_usecs; /* Relative microseconds (yes, it can be negative) */
    guint32      abs_secs;  /* Absolute seconds */
    guint32      abs_usecs; /* Absolute microseconds */
!   gint32       del_secs;  /* Delta seconds (yes, it can be negative) */
!   gint32       del_usecs; /* Delta microseconds (yes, it can be negative) */
    long         file_off;  /* File offset */
    column_info *cinfo;     /* Column formatting information */
    int          lnk_t;     /* Per-packet encapsulation/data-link type */
Index: tethereal.c
===================================================================
RCS file: /usr/local/cvsroot/ethereal/tethereal.c,v
retrieving revision 1.44
diff -c -r1.44 tethereal.c
*** tethereal.c	2000/08/23 18:21:57	1.44
--- tethereal.c	2000/09/10 06:35:40
***************
*** 821,835 ****
      firstusec = fdata->abs_usecs;
    }
  
-   /* Get the time elapsed between the first packet and this packet. */
-   cf->esec = fdata->abs_secs - firstsec;
-   if (firstusec <= fdata->abs_usecs) {
-     cf->eusec = fdata->abs_usecs - firstusec;
-   } else {
-     cf->eusec = (fdata->abs_usecs + 1000000) - firstusec;
-     cf->esec--;
-   }
-   
    /* If we don't have the time stamp of the previous displayed packet,
       it's because this is the first displayed packet.  Save the time
       stamp of this packet as the time stamp of the previous displayed
--- 821,826 ----
***************
*** 840,857 ****
    }
  
    /* Get the time elapsed between the first packet and this packet. */
!   fdata->rel_secs = cf->esec;
!   fdata->rel_usecs = cf->eusec;
    
    /* Get the time elapsed between the previous displayed packet and
       this packet. */
    fdata->del_secs = fdata->abs_secs - prevsec;
-   if (prevusec <= fdata->abs_usecs) {
-     fdata->del_usecs = fdata->abs_usecs - prevusec;
-   } else {
-     fdata->del_usecs = (fdata->abs_usecs + 1000000) - prevusec;
-     fdata->del_secs--;
-   }
    prevsec = fdata->abs_secs;
    prevusec = fdata->abs_usecs;
  
--- 831,853 ----
    }
  
    /* Get the time elapsed between the first packet and this packet. */
!   compute_timestamp_diff(&fdata->rel_secs, &fdata->rel_usecs,
! 		fdata->abs_secs, fdata->abs_usecs, firstsec, firstusec);
! 
!   /* If it's greater than the current elapsed time, set the elapsed time
!      to it (we check for "greater than" so as not to be confused by
!      time moving backwards). */
!   if (cf->esec < fdata->rel_secs
! 	|| (cf->esec == fdata->rel_secs && cf->eusec < fdata->rel_usecs)) {
!     cf->esec = fdata->rel_secs;
!     cf->eusec = fdata->rel_usecs;
!   }
    
    /* Get the time elapsed between the previous displayed packet and
       this packet. */
+   compute_timestamp_diff(&fdata->del_secs, &fdata->del_usecs,
+ 		fdata->abs_secs, fdata->abs_usecs, prevsec, prevusec);
    fdata->del_secs = fdata->abs_secs - prevsec;
    prevsec = fdata->abs_secs;
    prevusec = fdata->abs_usecs;
  
Index: util.c
===================================================================
RCS file: /usr/local/cvsroot/ethereal/util.c,v
retrieving revision 1.43
diff -c -r1.43 util.c
*** util.c	2000/09/07 09:57:39	1.43
--- util.c	2000/09/10 06:35:42
***************
*** 804,806 ****
--- 804,850 ----
  
  	return home;
  }
+ 
+ /* Compute the difference between two seconds/microseconds time stamps. */
+ void
+ compute_timestamp_diff(gint *diffsec, gint *diffusec,
+ 	guint32 sec1, guint32 usec1, guint32 sec2, guint32 usec2)
+ {
+   if (sec1 == sec2) {
+     /* The seconds part of the first time is the same as the seconds
+        part of the second time, so if the microseconds part of the first
+        time is less than the microseconds part of the second time, the
+        first time is before the second time.  The microseconds part of
+        the delta should just be the difference between the microseconds
+        part of the first time and the microseconds part of the second
+        time; don't adjust the seconds part of the delta, as it's OK if
+        the microseconds part is negative. */
+ 
+     *diffsec = sec1 - sec2;
+     *diffusec = usec1 - usec2;
+   } else if (sec1 <= sec2) {
+     /* The seconds part of the first time is less than the seconds part
+        of the second time, so the first time is before the second time.
+ 
+        Both the "seconds" and "microseconds" value of the delta
+        should have the same sign, so if the difference between the
+        microseconds values would be *positive*, subtract 1,000,000
+        from it, and add one to the seconds value. */
+     *diffsec = sec1 - sec2;
+     if (usec2 >= usec1) {
+       *diffusec = usec1 - usec2;
+     } else {
+       *diffusec = (usec1 - 1000000) - usec2;
+       (*diffsec)++;
+     }
+   } else {
+     /* Oh, good, we're not caught in a chronosynclastic infindibulum. */
+     *diffsec = sec1 - sec2;
+     if (usec2 <= usec1) {
+       *diffusec = usec1 - usec2;
+     } else {
+       *diffusec = (usec1 + 1000000) - usec2;
+       (*diffsec)--;
+     }
+   }
+ }
Index: util.h
===================================================================
RCS file: /usr/local/cvsroot/ethereal/util.h,v
retrieving revision 1.20
diff -c -r1.20 util.h
*** util.h	2000/07/31 04:53:32	1.20
--- util.h	2000/09/10 06:35:42
***************
*** 84,89 ****
--- 84,92 ----
  
  #endif
  
+ /* Compute the difference between two seconds/microseconds time stamps. */
+ void compute_timestamp_diff(gint *, gint *, guint32, guint32, guint32, guint32);
+ 
  #ifdef __cplusplus
  }
  #endif /* __cplusplus */