Add some higher-level file-read APIs and use them.

Add wtap_read_bytes(), which takes a FILE_T, a pointer, a byte count, an
error number pointer, and an error string pointer as arguments, and that
treats a short read of any sort, including a read that returns 0 bytes,
as a WTAP_ERR_SHORT_READ error, and that returns the error number and
string through its last two arguments.

Add wtap_read_bytes_or_eof(), which is similar, but that treats a read
that returns 0 bytes as an EOF, supplying an error number of 0 as an EOF
indication.

Use those in file readers; that simplifies the code and makes it less
likely that somebody will fail to supply the error number and error
string on a file read error.

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

1 files changed, 83 insertions, 97 deletions

diff --git a/wiretap/radcom.c b/wiretap/radcom.c
index 9b0072c1ff..bc19af68b4 100644
--- a/wiretap/radcom.c
+++ b/wiretap/radcom.c
@@ -54,7 +54,7 @@ static const guint8 encap_magic[4] = {
 };
 
 static const guint8 active_time_magic[11] = {
-	0x41, 0x63, 0x74, 0x69, 0x76, 0x65, 0x20, 0x54, 0x69, 0x6d, 0x65
+	'A', 'c', 't', 'i', 'v', 'e', ' ', 'T', 'i', 'm', 'e'
 };
 
 /* RADCOM record header - followed by frame data (perhaps including FCS).
@@ -90,12 +90,9 @@ static gboolean radcom_seek_read(wtap *wth, gint64 seek_off,
 	struct wtap_pkthdr *phdr, Buffer *buf, int *err, gchar **err_info);
 static gboolean radcom_read_rec(wtap *wth, FILE_T fh, struct wtap_pkthdr *phdr,
 	Buffer *buf, int *err, gchar **err_info);
-static gboolean radcom_read_rec_data(FILE_T fh, guint8 *pd, int length,
-	int *err, gchar **err_info);
 
 int radcom_open(wtap *wth, int *err, gchar **err_info)
 {
-	int bytes_read;
 	guint8 r_magic[8], t_magic[11], search_encap[7];
 	struct frame_date start_date;
 #if 0
@@ -105,10 +102,8 @@ int radcom_open(wtap *wth, int *err, gchar **err_info)
 
 	/* Read in the string that should be at the start of a RADCOM file */
 	errno = WTAP_ERR_CANT_READ;
-	bytes_read = file_read(r_magic, 8, wth->fh);
-	if (bytes_read != 8) {
-		*err = file_error(wth->fh, err_info);
-		if (*err != 0 && *err != WTAP_ERR_SHORT_READ)
+	if (!wtap_read_bytes(wth->fh, r_magic, 8, err, err_info)) {
+		if (*err != WTAP_ERR_SHORT_READ)
 			return -1;
 		return 0;
 	}
@@ -126,37 +121,89 @@ int radcom_open(wtap *wth, int *err, gchar **err_info)
 	/* Look for the "Active Time" string. The "frame_date" structure should
 	 * be located 32 bytes before the beginning of this string */
 	errno = WTAP_ERR_CANT_READ;
-	bytes_read = file_read(t_magic, 11, wth->fh);
-	if (bytes_read != 11) {
-		*err = file_error(wth->fh, err_info);
-		if (*err != 0 && *err != WTAP_ERR_SHORT_READ)
-			return -1;
-		return 0;
+	if (!wtap_read_bytes(wth->fh, t_magic, 11, err, err_info)) {
+		if (*err == WTAP_ERR_SHORT_READ) {
+			/*
+			 * Not enough bytes for the active time string,
+			 * so not a RADCOM file.
+			 */
+			return 0;
+		}
+		return -1;
 	}
 	while (memcmp(t_magic, active_time_magic, 11) != 0)
 	{
 		if (file_seek(wth->fh, -10, SEEK_CUR, err) == -1)
 			return -1;
 		errno = WTAP_ERR_CANT_READ;
-		bytes_read = file_read(t_magic, 11, wth->fh);
-		if (bytes_read != 11) {
-			*err = file_error(wth->fh, err_info);
-			if (*err != 0 && *err != WTAP_ERR_SHORT_READ)
-				return -1;
-			return 0;
+		if (!wtap_read_bytes(wth->fh, t_magic, 11, err, err_info)) {
+			if (*err == WTAP_ERR_SHORT_READ) {
+				/*
+				 * Not enough bytes for the active time string,
+				 * so not a RADCOM file.
+				 */
+				return 0;
+			}
+			return -1;
 		}
 	}
 	if (file_seek(wth->fh, -43, SEEK_CUR, err) == -1) return -1;
 
 	/* Get capture start time */
 	errno = WTAP_ERR_CANT_READ;
-	bytes_read = file_read(&start_date, sizeof(struct frame_date),
-			       wth->fh);
-	if (bytes_read != sizeof(struct frame_date)) {
-		*err = file_error(wth->fh, err_info);
-		if (*err != 0 && *err != WTAP_ERR_SHORT_READ)
+	if (!wtap_read_bytes(wth->fh, &start_date, sizeof(struct frame_date),
+	    err, err_info)) {
+		if (*err == WTAP_ERR_SHORT_READ) {
+			/*
+			 * Not enough bytes for the start time, so not
+			 * a RADCOM file.
+			 */
+			return 0;
+		}
+		return -1;
+	}
+
+	if (file_seek(wth->fh, sizeof(struct frame_date), SEEK_CUR, err) == -1)
+		return -1;
+
+	for (;;) {
+		errno = WTAP_ERR_CANT_READ;
+		if (!wtap_read_bytes(wth->fh, search_encap, 4,
+		    err, err_info)) {
+			if (*err == WTAP_ERR_SHORT_READ) {
+				/*
+				 * Not enough bytes for the encapsulation,
+				 * so not a RADCOM file.
+				 */
+				return 0;
+			}
+			return -1;
+		}
+
+		if (memcmp(encap_magic, search_encap, 4) == 0)
+			break;
+
+		/*
+		 * OK, that's not it, go forward 1 byte - reading
+		 * the magic moved us forward 4 bytes, so seeking
+		 * backward 3 bytes moves forward 1 byte - and
+		 * try the 4 bytes at that offset.
+		 */
+		if (file_seek(wth->fh, -3, SEEK_CUR, err) == -1)
 			return -1;
-		return 0;
+	}
+	if (file_seek(wth->fh, 12, SEEK_CUR, err) == -1)
+		return -1;
+	errno = WTAP_ERR_CANT_READ;
+	if (!wtap_read_bytes(wth->fh, search_encap, 4, err, err_info)) {
+		if (*err == WTAP_ERR_SHORT_READ) {
+			/*
+			 * Not enough bytes for the start time, so not
+			 * a RADCOM file.
+			 */
+			return 0;
+		}
+		return -1;
 	}
 
 	/* This is a radcom file */
@@ -176,30 +223,7 @@ int radcom_open(wtap *wth, int *err, gchar **err_info)
 	tm.tm_sec = sec%60;
 	tm.tm_isdst = -1;
 #endif
-	if (file_seek(wth->fh, sizeof(struct frame_date), SEEK_CUR, err) == -1)
-		return -1;
 
-	errno = WTAP_ERR_CANT_READ;
-	bytes_read = file_read(search_encap, 4, wth->fh);
-	if (bytes_read != 4) {
-		goto read_error;
-	}
-	while (memcmp(encap_magic, search_encap, 4)) {
-		if (file_seek(wth->fh, -3, SEEK_CUR, err) == -1)
-			return -1;
-		errno = WTAP_ERR_CANT_READ;
-		bytes_read = file_read(search_encap, 4, wth->fh);
-		if (bytes_read != 4) {
-			goto read_error;
-		}
-	}
-	if (file_seek(wth->fh, 12, SEEK_CUR, err) == -1)
-		return -1;
-	errno = WTAP_ERR_CANT_READ;
-	bytes_read = file_read(search_encap, 4, wth->fh);
-	if (bytes_read != 4) {
-		goto read_error;
-	}
 	if (memcmp(search_encap, "LAPB", 4) == 0)
 		wth->file_encap = WTAP_ENCAP_LAPB;
 	else if (memcmp(search_encap, "Ethe", 4) == 0)
@@ -213,21 +237,18 @@ int radcom_open(wtap *wth, int *err, gchar **err_info)
 	}
 
 #if 0
-	bytes_read = file_read(&next_date, sizeof(struct frame_date), wth->fh);
 	errno = WTAP_ERR_CANT_READ;
-	if (bytes_read != sizeof(struct frame_date)) {
-		goto read_error;
-	}
+	if (!wtap_read_bytes(wth->fh, &next_date, sizeof(struct frame_date),
+	    err, err_info))
+		return -1;
 
 	while (memcmp(&start_date, &next_date, 4)) {
 		if (file_seek(wth->fh, 1-sizeof(struct frame_date), SEEK_CUR, err) == -1)
 			return -1;
 		errno = WTAP_ERR_CANT_READ;
-		bytes_read = file_read(&next_date, sizeof(struct frame_date),
-				   wth->fh);
-		if (bytes_read != sizeof(struct frame_date)) {
-			goto read_error;
-		}
+		if (!wtap_read_bytes(wth->fh, &next_date, sizeof(struct frame_date),
+		    err, err_info))
+			return -1;
 	}
 #endif
 
@@ -243,19 +264,12 @@ int radcom_open(wtap *wth, int *err, gchar **err_info)
 	}
 
 	return 1;
-
-read_error:
-	*err = file_error(wth->fh, err_info);
-	if (*err != 0)
-		return -1;
-	return 0;
 }
 
 /* Read the next packet */
 static gboolean radcom_read(wtap *wth, int *err, gchar **err_info,
 			    gint64 *data_offset)
 {
-	int	bytes_read;
 	char	fcs[2];
 
 	*data_offset = file_tell(wth->fh);
@@ -273,13 +287,8 @@ static gboolean radcom_read(wtap *wth, int *err, gchar **err_info,
 		   presence and size of an FCS to our caller?
 		   That'd let us handle other file types as well. */
 		errno = WTAP_ERR_CANT_READ;
-		bytes_read = file_read(&fcs, sizeof fcs, wth->fh);
-		if (bytes_read != sizeof fcs) {
-			*err = file_error(wth->fh, err_info);
-			if (*err == 0)
-				*err = WTAP_ERR_SHORT_READ;
+		if (!wtap_read_bytes(wth->fh, &fcs, sizeof fcs, err, err_info))
 			return FALSE;
-		}
 	}
 
 	return TRUE;
@@ -311,20 +320,14 @@ radcom_read_rec(wtap *wth, FILE_T fh, struct wtap_pkthdr *phdr, Buffer *buf,
 		int *err, gchar **err_info)
 {
 	struct radcomrec_hdr hdr;
-	int	bytes_read;
 	guint16 data_length, real_length, length;
 	guint32 sec;
 	struct tm tm;
 	guint8	atmhdr[8];
 
 	errno = WTAP_ERR_CANT_READ;
-	bytes_read = file_read(&hdr, sizeof hdr, fh);
-	if (bytes_read != sizeof hdr) {
-		*err = file_error(fh, err_info);
-		if (*err == 0 && bytes_read != 0)
-			*err = WTAP_ERR_SHORT_READ;
+	if (!wtap_read_bytes_or_eof(fh, &hdr, sizeof hdr, err, err_info))
 		return FALSE;
-	}
 
 	data_length = pletoh16(&hdr.data_length);
 	if (data_length == 0) {
@@ -372,7 +375,8 @@ radcom_read_rec(wtap *wth, FILE_T fh, struct wtap_pkthdr *phdr, Buffer *buf,
 		 * XXX - is this stuff a pseudo-header?
 		 * The direction appears to be in the "hdr.dce" field.
 		 */
-		if (!radcom_read_rec_data(fh, atmhdr, sizeof atmhdr, err,
+		errno = WTAP_ERR_CANT_READ;
+		if (!wtap_read_bytes(fh, atmhdr, sizeof atmhdr, err,
 		    err_info))
 			return FALSE;	/* Read error */
 		length -= 8;
@@ -391,21 +395,3 @@ radcom_read_rec(wtap *wth, FILE_T fh, struct wtap_pkthdr *phdr, Buffer *buf,
 
 	return TRUE;
 }
-
-static gboolean
-radcom_read_rec_data(FILE_T fh, guint8 *pd, int length, int *err,
-		     gchar **err_info)
-{
-	int	bytes_read;
-
-	errno = WTAP_ERR_CANT_READ;
-	bytes_read = file_read(pd, length, fh);
-
-	if (bytes_read != length) {
-		*err = file_error(fh, err_info);
-		if (*err == 0)
-			*err = WTAP_ERR_SHORT_READ;
-		return FALSE;
-	}
-	return TRUE;
-}