[Asterisk-code-review] cleanup: Fix fread() and fwrite() error handling (asterisk[master])

Sean Bright asteriskteam at digium.com
Fri Apr 21 14:19:20 CDT 2017


Sean Bright has uploaded a new change for review. ( https://gerrit.asterisk.org/5512 )

Change subject: cleanup: Fix fread() and fwrite() error handling
......................................................................

cleanup: Fix fread() and fwrite() error handling

Cleaned up some of the incorrect uses of fread() and fwrite(), mostly in
the format modules. Neither of these functions will ever return a value
less than 0, which we were checking for in some cases.

I've introduced a fair amount of duplication in the format modules, but
I plan to change how format modules work internally in a subsequent
patch set, so this is simply a stop-gap.

I've also tried to document the places where fread() and fwrite() do not
behave as they are documented to behave because we are abusing FILE
pointers in the TCP/TLS code. We should really put forth an effort to
remove that code in future versions.

Change-Id: I8ca1cd47c20b2c0b72088bd13b9046f6977aa872
---
M addons/format_mp3.c
M apps/app_minivm.c
M apps/app_voicemail.c
M channels/chan_unistim.c
M formats/format_g719.c
M formats/format_g723.c
M formats/format_g726.c
M formats/format_g729.c
M formats/format_gsm.c
M formats/format_h263.c
M formats/format_h264.c
M formats/format_ilbc.c
M formats/format_ogg_vorbis.c
M formats/format_pcm.c
M formats/format_siren14.c
M formats/format_siren7.c
M formats/format_sln.c
M formats/format_vox.c
M formats/format_wav.c
M formats/format_wav_gsm.c
20 files changed, 216 insertions(+), 81 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/12/5512/1

diff --git a/addons/format_mp3.c b/addons/format_mp3.c
index e0f57b8..bb0b208 100644
--- a/addons/format_mp3.c
+++ b/addons/format_mp3.c
@@ -118,9 +118,11 @@
 
 	res = ftell(s->f);
 	p->sbuflen = fread(p->sbuf, 1, MP3_SCACHE, s->f);
-	if(p->sbuflen < 0) {
-		ast_log(LOG_WARNING, "Short read (%d) (%s)!\n", p->sbuflen, strerror(errno));
-		return -1;
+	if (p->sbuflen < MP3_SCACHE) {
+		if (ferror(s->f)) {
+			ast_log(LOG_WARNING, "Error while reading MP3 file: %s\n", strerror(errno));
+			return -1;
+		}
 	}
 	res = decodeMP3(&p->mp,p->sbuf,p->sbuflen,p->dbuf,MP3_DCACHE,&p->dbuflen);
 	if(res != MP3_OK)
diff --git a/apps/app_minivm.c b/apps/app_minivm.c
index 4cc2f47..9f8b13f 100644
--- a/apps/app_minivm.c
+++ b/apps/app_minivm.c
@@ -854,7 +854,7 @@
 	if (bio->ateof)
 		return 0;
 
-	if ((l = fread(bio->iobuf, 1, B64_BASEMAXINLINE,fi)) <= 0) {
+	if ((l = fread(bio->iobuf, 1, B64_BASEMAXINLINE, fi)) != B64_BASEMAXINLINE) {
 		if (ferror(fi))
 			return -1;
 
diff --git a/apps/app_voicemail.c b/apps/app_voicemail.c
index de82670..8d68996 100644
--- a/apps/app_voicemail.c
+++ b/apps/app_voicemail.c
@@ -2728,9 +2728,9 @@
 			*(vmu->email) = '\0';
 		return -1;
 	}
-	if (fread(buf, len, 1, p) < len) {
+	if (fread(buf, 1, len, p) != len) {
 		if (ferror(p)) {
-			ast_log(LOG_ERROR, "Short read while reading in mail file.\n");
+			ast_log(LOG_ERROR, "Error while reading mail file: %s\n");
 			return -1;
 		}
 	}
@@ -4743,7 +4743,7 @@
 	if (bio->ateof)
 		return 0;
 
-	if ((l = fread(bio->iobuf, 1, BASEMAXINLINE, fi)) <= 0) {
+	if ((l = fread(bio->iobuf, 1, BASEMAXINLINE, fi)) != BASEMAXINLINE) {
 		if (ferror(fi))
 			return -1;
 
diff --git a/channels/chan_unistim.c b/channels/chan_unistim.c
index b820cf5..9295ee4 100644
--- a/channels/chan_unistim.c
+++ b/channels/chan_unistim.c
@@ -2251,15 +2251,15 @@
 		display_last_error("Unable to write history log header.");
 		return -1;
 	}
-	if (fwrite(line1, TEXT_LENGTH_MAX, 1, f) != 1) {
+	if (fwrite(line1, 1, TEXT_LENGTH_MAX, f) != TEXT_LENGTH_MAX) {
 		display_last_error("Unable to write history entry - date.");
 		return -1;
 	}
-	if (fwrite(pte->device->lst_cid, TEXT_LENGTH_MAX, 1, f) != 1) {
+	if (fwrite(pte->device->lst_cid, 1, TEXT_LENGTH_MAX, f) != TEXT_LENGTH_MAX) {
 		display_last_error("Unable to write history entry - callerid.");
 		return -1;
 	}
-	if (fwrite(pte->device->lst_cnm, TEXT_LENGTH_MAX, 1, f) != 1) {
+	if (fwrite(pte->device->lst_cnm, 1, TEXT_LENGTH_MAX, f) != TEXT_LENGTH_MAX) {
 		display_last_error("Unable to write history entry - callername.");
 		return -1;
 	}
@@ -2349,7 +2349,7 @@
 		}
 		memset(line1, ' ', TEXT_LENGTH_MAX);
 		for (i = 3; i < MAX_ENTRY_LOG * 3; i++) {
-			if (fwrite(line1, TEXT_LENGTH_MAX, 1, f) != 1) {
+			if (fwrite(line1, 1, TEXT_LENGTH_MAX, f) != TEXT_LENGTH_MAX) {
 				display_last_error("Unable to write history entry - stuffing.");
 				fclose(f);
 				return -1;
@@ -2395,14 +2395,16 @@
 		return -1;
 	}
 
-	if (fread(histbuf, size, 1, f) != 1) {
-		ast_free(histbuf);
-		fclose(f);
-		fclose(f2);
-		display_last_error("Unable to read previous history entries.");
-		return -1;
+	if (fread(histbuf, 1, size, f) != size) {
+		if (ferror(f)) {
+			ast_free(histbuf);
+			fclose(f);
+			fclose(f2);
+			display_last_error("Unable to read previous history entries.");
+			return -1;
+		}
 	}
-	if (fwrite(histbuf, size, 1, f2) != 1) {
+	if (fwrite(histbuf, 1, size, f2) != size) {
 		ast_free(histbuf);
 		fclose(f);
 		fclose(f2);
@@ -3954,10 +3956,12 @@
 		func3[10];
 
 	/* Display date/time and call status */
-	if (fread(line, TEXT_LENGTH_MAX, 1, *f) != 1) {
-		display_last_error("Can't read history date entry");
-		fclose(*f);
-		return;
+	if (fread(line, 1, TEXT_LENGTH_MAX, *f) != TEXT_LENGTH_MAX) {
+		if (ferror(*f)) {
+			display_last_error("Can't read history date entry");
+			fclose(*f);
+			return;
+		}
 	}
 	line[sizeof(line) - 1] = '\0';
 	if (pte->device->height == 1) {
@@ -3968,10 +3972,12 @@
 		send_text(TEXT_LINE0, TEXT_NORMAL, pte, line);
 	}
 	/* Display number */
-	if (fread(line, TEXT_LENGTH_MAX, 1, *f) != 1) {
-		display_last_error("Can't read callerid entry");
-		fclose(*f);
-		return;
+	if (fread(line, 1, TEXT_LENGTH_MAX, *f) != TEXT_LENGTH_MAX) {
+		if (ferror(*f)) {
+			display_last_error("Can't read callerid entry");
+			fclose(*f);
+			return;
+		}
 	}
 	line[sizeof(line) - 1] = '\0';
 	ast_copy_string(pte->device->lst_cid, line, sizeof(pte->device->lst_cid));
@@ -3984,10 +3990,12 @@
 		send_text(TEXT_LINE1, TEXT_NORMAL, pte, line);
 	}
 	/* Display name */
-	if (fread(line, TEXT_LENGTH_MAX, 1, *f) != 1) {
-		display_last_error("Can't read callername entry");
-		fclose(*f);
-		return;
+	if (fread(line, 1, TEXT_LENGTH_MAX, *f) != TEXT_LENGTH_MAX) {
+		if (ferror(*f)) {
+			display_last_error("Can't read callername entry");
+			fclose(*f);
+			return;
+		}
 	}
 	line[sizeof(line) - 1] = '\0';
 	if (pte->device->height == 1) {
diff --git a/formats/format_g719.c b/formats/format_g719.c
index 8cc9427..572b88f 100644
--- a/formats/format_g719.c
+++ b/formats/format_g719.c
@@ -45,8 +45,16 @@
 
 	AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, BUF_SIZE);
 	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) != s->fr.datalen) {
-		if (res)
-			ast_log(LOG_WARNING, "Short read (%d) (%s)!\n", res, strerror(errno));
+		if (feof(s->f)) {
+			if (res) {
+				ast_log(LOG_WARNING, "Incomplete frame data at end of %s file "
+						"(expected %d bytes, read %d)\n",
+						ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res);
+			}
+		} else {
+			ast_log(LOG_ERROR, "Error while reading %s file: %s\n",
+					ast_format_get_name(s->fr.subclass.format), strerror(errno));
+		}
 		return NULL;
 	}
 	*whennext = s->fr.samples = BYTES_TO_SAMPLES(res);
diff --git a/formats/format_g723.c b/formats/format_g723.c
index 04e03b6..d4c4d4b 100644
--- a/formats/format_g723.c
+++ b/formats/format_g723.c
@@ -64,8 +64,17 @@
 	}
 	/* Read the data into the buffer */
 	AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, size);
-	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) != size) {
-		ast_log(LOG_WARNING, "Short read (%d of %d bytes) (%s)!\n", res, size, strerror(errno));
+	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) != s->fr.datalen) {
+		if (feof(s->f)) {
+			if (res) {
+				ast_log(LOG_WARNING, "Incomplete frame data at end of %s file "
+						"(expected %d bytes, read %d)\n",
+						ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res);
+			}
+		} else {
+			ast_log(LOG_ERROR, "Error while reading %s file: %s\n",
+					ast_format_get_name(s->fr.subclass.format), strerror(errno));
+		}
 		return NULL;
 	}
 	*whennext = s->fr.samples = 240;
diff --git a/formats/format_g726.c b/formats/format_g726.c
index 08e669e..7da6d1e 100644
--- a/formats/format_g726.c
+++ b/formats/format_g726.c
@@ -124,8 +124,16 @@
 	AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, frame_size[fs->rate]);
 	s->fr.samples = 8 * FRAME_TIME;
 	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) != s->fr.datalen) {
-		if (res)
-			ast_log(LOG_WARNING, "Short read (%d) (%s)!\n", res, strerror(errno));
+		if (feof(s->f)) {
+			if (res) {
+				ast_log(LOG_WARNING, "Incomplete frame data at end of %s file "
+						"(expected %d bytes, read %d)\n",
+						ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res);
+			}
+		} else {
+			ast_log(LOG_ERROR, "Error while reading %s file: %s\n",
+					ast_format_get_name(s->fr.subclass.format), strerror(errno));
+		}
 		return NULL;
 	}
 	*whennext = s->fr.samples;
diff --git a/formats/format_g729.c b/formats/format_g729.c
index 49e5802..4cefc04 100644
--- a/formats/format_g729.c
+++ b/formats/format_g729.c
@@ -51,8 +51,16 @@
 	s->fr.samples = G729A_SAMPLES;
 	AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, BUF_SIZE);
 	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) != s->fr.datalen) {
-		if (res && (res != 10))	/* XXX what for ? */
-			ast_log(LOG_WARNING, "Short read (%d) (%s)!\n", res, strerror(errno));
+		if (feof(s->f)) {
+			if (res) {
+				ast_log(LOG_WARNING, "Incomplete frame data at end of %s file "
+						"(expected %d bytes, read %d)\n",
+						ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res);
+			}
+		} else {
+			ast_log(LOG_ERROR, "Error while reading %s file: %s\n",
+					ast_format_get_name(s->fr.subclass.format), strerror(errno));
+		}
 		return NULL;
 	}
 	*whennext = s->fr.samples;
diff --git a/formats/format_gsm.c b/formats/format_gsm.c
index a2b6d36..39deb98 100644
--- a/formats/format_gsm.c
+++ b/formats/format_gsm.c
@@ -55,10 +55,18 @@
 {
 	int res;
 
-	AST_FRAME_SET_BUFFER(&(s->fr), s->buf, AST_FRIENDLY_OFFSET, GSM_FRAME_SIZE)
+	AST_FRAME_SET_BUFFER(&(s->fr), s->buf, AST_FRIENDLY_OFFSET, GSM_FRAME_SIZE);
 	if ((res = fread(s->fr.data.ptr, 1, GSM_FRAME_SIZE, s->f)) != GSM_FRAME_SIZE) {
-		if (res)
-			ast_log(LOG_WARNING, "Short read (%d) (%s)!\n", res, strerror(errno));
+		if (feof(s->f)) {
+			if (res) {
+				ast_log(LOG_WARNING, "Incomplete frame data at end of %s file "
+						"(expected %d bytes, read %d)\n",
+						ast_format_get_name(s->fr.subclass.format), GSM_FRAME_SIZE, res);
+			}
+		} else {
+			ast_log(LOG_ERROR, "Error while reading %s file: %s\n",
+					ast_format_get_name(s->fr.subclass.format), strerror(errno));
+		}
 		return NULL;
 	}
 	*whennext = s->fr.samples = GSM_SAMPLES;
@@ -131,7 +139,7 @@
 		int i;
 		fseeko(fs->f, 0, SEEK_END);
 		for (i=0; i< (offset - max) / GSM_FRAME_SIZE; i++) {
-			if (!fwrite(gsm_silence, 1, GSM_FRAME_SIZE, fs->f)) {
+			if (fwrite(gsm_silence, 1, GSM_FRAME_SIZE, fs->f) != GSM_FRAME_SIZE) {
 				ast_log(LOG_WARNING, "fwrite() failed: %s\n", strerror(errno));
 			}
 		}
diff --git a/formats/format_h263.c b/formats/format_h263.c
index 4cc3db5..d05e598 100644
--- a/formats/format_h263.c
+++ b/formats/format_h263.c
@@ -58,7 +58,7 @@
 {
 	unsigned int ts;
 
-	if (fread(&ts, 1, sizeof(ts), s->f) < sizeof(ts)) {
+	if (fread(&ts, 1, sizeof(ts), s->f) != sizeof(ts)) {
 		ast_log(LOG_WARNING, "Empty file!\n");
 		return -1;
 	}
@@ -74,7 +74,7 @@
 	struct h263_desc *fs = (struct h263_desc *)s->_private;
 
 	/* Send a frame from the file to the appropriate channel */
-	if ((res = fread(&len, 1, sizeof(len), s->f)) < 1)
+	if ((res = fread(&len, 1, sizeof(len), s->f)) != sizeof(len))
 		return NULL;
 	len = ntohs(len);
 	mark = (len & FRAME_ENDED) ? 1 : 0;
@@ -85,8 +85,16 @@
 	}
 	AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, len);
 	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) != s->fr.datalen) {
-		if (res)
-			ast_log(LOG_WARNING, "Short read (%d) (%s)!\n", res, strerror(errno));
+		if (feof(s->f)) {
+			if (res) {
+				ast_log(LOG_WARNING, "Incomplete frame data at end of %s file "
+						"(expected %d bytes, read %d)\n",
+						ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res);
+			}
+		} else {
+			ast_log(LOG_ERROR, "Error while reading %s file: %s\n",
+					ast_format_get_name(s->fr.subclass.format), strerror(errno));
+		}
 		return NULL;
 	}
 	s->fr.samples = fs->lastts;	/* XXX what ? */
diff --git a/formats/format_h264.c b/formats/format_h264.c
index 60b0902..47f71ae 100644
--- a/formats/format_h264.c
+++ b/formats/format_h264.c
@@ -50,7 +50,7 @@
 static int h264_open(struct ast_filestream *s)
 {
 	unsigned int ts;
-	if (fread(&ts, 1, sizeof(ts), s->f) < sizeof(ts)) {
+	if (fread(&ts, 1, sizeof(ts), s->f) != sizeof(ts)) {
 		ast_log(LOG_WARNING, "Empty file!\n");
 		return -1;
 	}
@@ -66,7 +66,7 @@
 	struct h264_desc *fs = (struct h264_desc *)s->_private;
 
 	/* Send a frame from the file to the appropriate channel */
-	if ((res = fread(&len, 1, sizeof(len), s->f)) < 1)
+	if ((res = fread(&len, 1, sizeof(len), s->f)) != sizeof(len))
 		return NULL;
 	len = ntohs(len);
 	mark = (len & FRAME_ENDED) ? 1 : 0;
@@ -77,8 +77,16 @@
 	}
 	AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, len);
 	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) != s->fr.datalen) {
-		if (res)
-			ast_log(LOG_WARNING, "Short read (%d of %d) (%s)!\n", res, len, strerror(errno));
+		if (feof(s->f)) {
+			if (res) {
+				ast_log(LOG_WARNING, "Incomplete frame data at end of %s file "
+						"(expected %d bytes, read %d)\n",
+						ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res);
+			}
+		} else {
+			ast_log(LOG_ERROR, "Error while reading %s file: %s\n",
+					ast_format_get_name(s->fr.subclass.format), strerror(errno));
+		}
 		return NULL;
 	}
 	s->fr.samples = fs->lastts;
diff --git a/formats/format_ilbc.c b/formats/format_ilbc.c
index eab465d..ec8ad0f 100644
--- a/formats/format_ilbc.c
+++ b/formats/format_ilbc.c
@@ -49,8 +49,16 @@
 	/* Send a frame from the file to the appropriate channel */
 	AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, ILBC_BUF_SIZE);
 	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) != s->fr.datalen) {
-		if (res)
-			ast_log(LOG_WARNING, "Short read (%d) (%s)!\n", res, strerror(errno));
+		if (feof(s->f)) {
+			if (res) {
+				ast_log(LOG_WARNING, "Incomplete frame data at end of %s file "
+						"(expected %d bytes, read %d)\n",
+						ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res);
+			}
+		} else {
+			ast_log(LOG_ERROR, "Error while reading %s file: %s\n",
+					ast_format_get_name(s->fr.subclass.format), strerror(errno));
+		}
 		return NULL;
 	}
 	*whennext = s->fr.samples = ILBC_SAMPLES;
diff --git a/formats/format_ogg_vorbis.c b/formats/format_ogg_vorbis.c
index c0f8c19..4fdd1c4 100644
--- a/formats/format_ogg_vorbis.c
+++ b/formats/format_ogg_vorbis.c
@@ -181,10 +181,10 @@
 	while (!tmp->eos) {
 		if (ogg_stream_flush(&tmp->os, &tmp->og) == 0)
 			break;
-		if (!fwrite(tmp->og.header, 1, tmp->og.header_len, s->f)) {
+		if (fwrite(tmp->og.header, 1, tmp->og.header_len, s->f) != tmp->og.header_len) {
 			ast_log(LOG_WARNING, "fwrite() failed: %s\n", strerror(errno));
 		}
-		if (!fwrite(tmp->og.body, 1, tmp->og.body_len, s->f)) {
+		if (fwrite(tmp->og.body, 1, tmp->og.body_len, s->f) != tmp->og.body_len) {
 			ast_log(LOG_WARNING, "fwrite() failed: %s\n", strerror(errno));
 		}
 		if (ogg_page_eos(&tmp->og))
@@ -211,10 +211,10 @@
 				if (ogg_stream_pageout(&s->os, &s->og) == 0) {
 					break;
 				}
-				if (!fwrite(s->og.header, 1, s->og.header_len, f)) {
-				ast_log(LOG_WARNING, "fwrite() failed: %s\n", strerror(errno));
+				if (fwrite(s->og.header, 1, s->og.header_len, f) != s->og.header_len) {
+					ast_log(LOG_WARNING, "fwrite() failed: %s\n", strerror(errno));
 				}
-				if (!fwrite(s->og.body, 1, s->og.body_len, f)) {
+				if (fwrite(s->og.body, 1, s->og.body_len, f) != s->og.body_len) {
 					ast_log(LOG_WARNING, "fwrite() failed: %s\n", strerror(errno));
 				}
 				if (ogg_page_eos(&s->og)) {
diff --git a/formats/format_pcm.c b/formats/format_pcm.c
index 97af0e9..f5ddda9 100644
--- a/formats/format_pcm.c
+++ b/formats/format_pcm.c
@@ -83,9 +83,17 @@
 	/* Send a frame from the file to the appropriate channel */
 
 	AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, BUF_SIZE);
-	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) < 1) {
-		if (res)
-			ast_log(LOG_WARNING, "Short read (%d) (%s)!\n", res, strerror(errno));
+	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) != s->fr.datalen) {
+		if (feof(s->f)) {
+			if (res) {
+				ast_log(LOG_WARNING, "Incomplete frame data at end of %s file "
+						"(expected %d bytes, read %d)\n",
+						ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res);
+			}
+		} else {
+			ast_log(LOG_ERROR, "Error while reading %s file: %s\n",
+					ast_format_get_name(s->fr.subclass.format), strerror(errno));
+		}
 		return NULL;
 	}
 	s->fr.datalen = res;
@@ -140,9 +148,10 @@
 		const char *src = (ast_format_cmp(fs->fmt->format, ast_format_alaw) == AST_FORMAT_CMP_EQUAL) ? alaw_silence : ulaw_silence;
 
 		while (left) {
-			size_t written = fwrite(src, 1, (left > BUF_SIZE) ? BUF_SIZE : left, fs->f);
-			if (written == -1)
+			size_t written = fwrite(src, 1, MIN(left, BUF_SIZE), fs->f);
+			if (written < MIN(left, BUF_SIZE)) {
 				break;	/* error */
+			}
 			left -= written;
 		}
 		ret = 0; /* successful */
@@ -210,7 +219,10 @@
 				to_write = fpos - cur;
 				if (to_write > sizeof(buf))
 					to_write = sizeof(buf);
-				fwrite(buf, 1, to_write, fs->f);
+				if (fwrite(buf, 1, to_write, fs->f) != to_write) {
+					ast_log(LOG_ERROR, "Failed to write to file: %s\n", strerror(errno));
+					return -1;
+				}
 				cur += to_write;
 			}
 		}
diff --git a/formats/format_siren14.c b/formats/format_siren14.c
index 1ce7d18..d54ed99 100644
--- a/formats/format_siren14.c
+++ b/formats/format_siren14.c
@@ -45,8 +45,16 @@
 
 	AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, BUF_SIZE);
 	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) != s->fr.datalen) {
-		if (res)
-			ast_log(LOG_WARNING, "Short read (%d) (%s)!\n", res, strerror(errno));
+		if (feof(s->f)) {
+			if (res) {
+				ast_log(LOG_WARNING, "Incomplete frame data at end of %s file "
+						"(expected %d bytes, read %d)\n",
+						ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res);
+			}
+		} else {
+			ast_log(LOG_ERROR, "Error while reading %s file: %s\n",
+					ast_format_get_name(s->fr.subclass.format), strerror(errno));
+		}
 		return NULL;
 	}
 	*whennext = s->fr.samples = BYTES_TO_SAMPLES(res);
diff --git a/formats/format_siren7.c b/formats/format_siren7.c
index d205984..f3b4b42 100644
--- a/formats/format_siren7.c
+++ b/formats/format_siren7.c
@@ -45,8 +45,16 @@
 
 	AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, BUF_SIZE);
 	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) != s->fr.datalen) {
-		if (res)
-			ast_log(LOG_WARNING, "Short read (%d) (%s)!\n", res, strerror(errno));
+		if (feof(s->f)) {
+			if (res) {
+				ast_log(LOG_WARNING, "Incomplete frame data at end of %s file "
+						"(expected %d bytes, read %d)\n",
+						ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res);
+			}
+		} else {
+			ast_log(LOG_ERROR, "Error while reading %s file: %s\n",
+					ast_format_get_name(s->fr.subclass.format), strerror(errno));
+		}
 		return NULL;
 	}
 	*whennext = s->fr.samples = BYTES_TO_SAMPLES(res);
diff --git a/formats/format_sln.c b/formats/format_sln.c
index af3f691..1977f7d 100644
--- a/formats/format_sln.c
+++ b/formats/format_sln.c
@@ -38,9 +38,17 @@
 	/* Send a frame from the file to the appropriate channel */
 
 	AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, buf_size);
-	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) < 1) {
-		if (res)
-			ast_log(LOG_WARNING, "Short read (%d) (%s)!\n", res, strerror(errno));
+	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) != s->fr.datalen) {
+		if (feof(s->f)) {
+			if (res) {
+				ast_log(LOG_WARNING, "Incomplete frame data at end of %s file "
+						"(expected %d bytes, read %d)\n",
+						ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res);
+			}
+		} else {
+			ast_log(LOG_ERROR, "Error while reading %s file: %s\n",
+					ast_format_get_name(s->fr.subclass.format), strerror(errno));
+		}
 		return NULL;
 	}
 	*whennext = s->fr.samples = res/2;
diff --git a/formats/format_vox.c b/formats/format_vox.c
index 5a70c34..195714c 100644
--- a/formats/format_vox.c
+++ b/formats/format_vox.c
@@ -44,9 +44,17 @@
 
 	/* Send a frame from the file to the appropriate channel */
 	AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, BUF_SIZE);
-	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) < 1) {
-		if (res)
-			ast_log(LOG_WARNING, "Short read (%d) (%s)!\n", res, strerror(errno));
+	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) != s->fr.datalen) {
+		if (feof(s->f)) {
+			if (res) {
+				ast_log(LOG_WARNING, "Incomplete frame data at end of %s file "
+						"(expected %d bytes, read %d)\n",
+						ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res);
+			}
+		} else {
+			ast_log(LOG_ERROR, "Error while reading %s file: %s\n",
+					ast_format_get_name(s->fr.subclass.format), strerror(errno));
+		}
 		return NULL;
 	}
 	*whennext = s->fr.samples = res * 2;
diff --git a/formats/format_wav.c b/formats/format_wav.c
index 8316c35..ee3db17 100644
--- a/formats/format_wav.c
+++ b/formats/format_wav.c
@@ -361,7 +361,7 @@
 
 	/* Pad to even length */
 	if (fs->bytes & 0x1) {
-		if (!fwrite(&zero, 1, 1, s->f)) {
+		if (fwrite(&zero, 1, 1, s->f) != 1) {
 			ast_log(LOG_WARNING, "fwrite() failed: %s\n", strerror(errno));
 		}
 	}
@@ -389,10 +389,18 @@
 		bytes = 0;
 /* 	ast_debug(1, "here: %d, maxlen: %d, bytes: %d\n", here, s->maxlen, bytes); */
 	AST_FRAME_SET_BUFFER(&s->fr, s->buf, AST_FRIENDLY_OFFSET, bytes);
-	
-	if ( (res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) <= 0 ) {
-		if (res)
-			ast_log(LOG_WARNING, "Short read (%d) (%s)!\n", res, strerror(errno));
+
+	if ((res = fread(s->fr.data.ptr, 1, s->fr.datalen, s->f)) != s->fr.datalen) {
+		if (feof(s->f)) {
+			if (res) {
+				ast_log(LOG_WARNING, "Incomplete frame data at end of %s file "
+						"(expected %d bytes, read %d)\n",
+						ast_format_get_name(s->fr.subclass.format), s->fr.datalen, res);
+			}
+		} else {
+			ast_log(LOG_ERROR, "Error while reading %s file: %s\n",
+					ast_format_get_name(s->fr.subclass.format), strerror(errno));
+		}
 		return NULL;
 	}
 	s->fr.datalen = res;
diff --git a/formats/format_wav_gsm.c b/formats/format_wav_gsm.c
index eef06ce..bfec903 100644
--- a/formats/format_wav_gsm.c
+++ b/formats/format_wav_gsm.c
@@ -422,8 +422,16 @@
 		int res;
 		
 		if ((res = fread(msdata, 1, MSGSM_FRAME_SIZE, s->f)) != MSGSM_FRAME_SIZE) {
-			if (res && (res != 1))
-				ast_log(LOG_WARNING, "Short read (%d) (%s)!\n", res, strerror(errno));
+			if (feof(s->f)) {
+				if (res) {
+					ast_log(LOG_WARNING, "Incomplete frame data at end of %s file "
+							"(expected %d bytes, read %d)\n",
+							ast_format_get_name(s->fr.subclass.format), MSGSM_FRAME_SIZE, res);
+				}
+			} else {
+				ast_log(LOG_ERROR, "Error while reading %s file: %s\n",
+						ast_format_get_name(s->fr.subclass.format), strerror(errno));
+			}
 			return NULL;
 		}
 		/* Convert from MS format to two real GSM frames */
@@ -511,7 +519,7 @@
 		int i;
 		fseek(fs->f, 0, SEEK_END);
 		for (i=0; i< (offset - max) / MSGSM_FRAME_SIZE; i++) {
-			if (!fwrite(msgsm_silence, 1, MSGSM_FRAME_SIZE, fs->f)) {
+			if (fwrite(msgsm_silence, 1, MSGSM_FRAME_SIZE, fs->f) != MSGSM_FRAME_SIZE) {
 				ast_log(LOG_WARNING, "fwrite() failed: %s\n", strerror(errno));
 			}
 		}

-- 
To view, visit https://gerrit.asterisk.org/5512
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I8ca1cd47c20b2c0b72088bd13b9046f6977aa872
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>



More information about the asterisk-code-review mailing list