Move the bitrate test against 0 to mp2t_bits_per_second().

As the comment says, that routine "[ensures] there is a valid bitrate",
and a bitrate of 0, which comes from truncating a fractional bitrate, is
not a valid bitrate (an MPEG-2 Transport Stream with a bitrate less than
1 bit per second is not going to carrry any sensible audio/video
stream).

Make the "first" argument unsigned; it can never be negative.
Restructure the code and change some data types to make it more obvious
that it can't.

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

1 files changed, 26 insertions, 18 deletions

diff --git a/wiretap/mp2t.c b/wiretap/mp2t.c
index 6004bc6475..a569c99c05 100644
--- a/wiretap/mp2t.c
+++ b/wiretap/mp2t.c
@@ -54,7 +54,7 @@
 
 
 typedef struct {
-    int start_offset;
+    guint32 start_offset;
     guint64 bitrate;
     /* length of trailing data (e.g. FEC) that's appended after each packet */
     guint8  trailer_len;
@@ -211,7 +211,7 @@ mp2t_find_next_pcr(wtap *wth, guint8 trailer_len,
 }
 
 static wtap_open_return_val
-mp2t_bits_per_second(wtap *wth, gint first, guint8 trailer_len,
+mp2t_bits_per_second(wtap *wth, guint32 first, guint8 trailer_len,
         guint64 *bitrate, int *err, gchar **err_info)
 {
     guint32 pn1, pn2;
@@ -228,10 +228,7 @@ mp2t_bits_per_second(wtap *wth, gint first, guint8 trailer_len,
      * XXX - is this assuming that the time stamps in the PCRs correspond
      * to the time scale of the underlying transport stream?
      */
-
-    if (first<0)
-        return WTAP_OPEN_ERROR;
-    idx = (guint32)first;
+    idx = first;
 
     if (!mp2t_find_next_pcr(wth, trailer_len, err, err_info, &idx, &pcr1, &pid1)) {
         /* Read error, short read, or EOF */
@@ -281,6 +278,21 @@ mp2t_bits_per_second(wtap *wth, gint first, guint8 trailer_len,
     bits_passed = MP2T_SIZE * (pn2 - pn1) * 8;
 
     *bitrate = ((MP2T_PCR_CLOCK * bits_passed) / pcr_delta);
+    if (*bitrate == 0) {
+        /* pcr_delta < MP2T_PCR_CLOCK * bits_passed (pn2 != pn1,
+         * as that's the test for the loop above, so bits_passed
+         * is non-zero).
+         *
+         * That will produce a fractional bitrate, which turns
+         * into zero, causing a zero divide later.
+         *
+         * XXX - should we report this as "not ours"?  A bitrate
+         * of less than 1 bit per second is not very useful for any
+         * form of audio/video, so presumably that's unlikely to
+         * be an MP2T file.
+         */
+        return WTAP_OPEN_ERROR;
+    }
     return WTAP_OPEN_MINE;
 }
 
@@ -290,8 +302,8 @@ mp2t_open(wtap *wth, int *err, gchar **err_info)
     guint8 buffer[MP2T_SIZE+TRAILER_LEN_MAX];
     guint8 trailer_len = 0;
     guint sync_steps = 0;
-    int i;
-    int first;
+    guint i;
+    guint32 first = 0;
     mp2t_filetype_t *mp2t;
     wtap_open_return_val status;
     guint64 bitrate;
@@ -303,17 +315,18 @@ mp2t_open(wtap *wth, int *err, gchar **err_info)
         return WTAP_OPEN_NOT_MINE;
     }
 
-    first = -1;
     for (i = 0; i < MP2T_SIZE; i++) {
         if (MP2T_SYNC_BYTE == buffer[i]) {
             first = i;
-            break;
+            goto found;
         }
     }
-    if (-1 == first) {
-        return WTAP_OPEN_NOT_MINE; /* wrong file type - not an mpeg2 ts file */
-    }
+    /*
+     * No sync bytes found, so not an MPEG-2 Transport Stream file.
+     */
+    return WTAP_OPEN_NOT_MINE; /* wrong file type - not an mpeg2 ts file */
 
+found:
     if (-1 == file_seek(wth->fh, first, SEEK_SET, err)) {
         return WTAP_OPEN_ERROR;
     }
@@ -366,11 +379,6 @@ mp2t_open(wtap *wth, int *err, gchar **err_info)
         return status;
     }
 
-    if (bitrate == 0) {
-        /* Prevent an eventual divide by zero */
-        return WTAP_OPEN_ERROR;
-    }
-
     if (-1 == file_seek(wth->fh, first, SEEK_SET, err)) {
         return WTAP_OPEN_ERROR;
     }