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>
This commit is contained in:
Guy Harris 2015-12-01 16:35:00 -08:00
parent d2644aef36
commit 266d3b7d51
1 changed files with 26 additions and 18 deletions

View File

@ -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;
}