From: Andre Noll <maan@tuebingen.mpg.de>
Date: Tue, 24 Aug 2021 12:49:16 +0000 (+0200)
Subject: mp4: Implement error checking for the write path.
X-Git-Tag: v0.7.1~7^2~25
X-Git-Url: http://git.tue.mpg.de/?a=commitdiff_plain;h=d440a71683940a58747de6dc32643db452d9cf54;p=paraslash.git

mp4: Implement error checking for the write path.

Although the ->write callback has a return value, it is of unsigned
type and is never checked. Fix this by changing the prototype to
match that of the write(2) system call, check the return value of the
callback in the write_data() wrapper of mp4.c and propagate paraslash
error codes back to aac_afh.c via the public mp4_meta_update().

While at it, handle short writes and EINTR properly, and fix the
indentation of the callback structure in mp4.h.
---

diff --git a/aac_afh.c b/aac_afh.c
index f3a06c4c..0a80bfcf 100644
--- a/aac_afh.c
+++ b/aac_afh.c
@@ -173,10 +173,10 @@ static uint32_t aac_afh_meta_seek_cb(void *user_data, uint64_t pos)
 	return lseek(fd, pos, SEEK_SET);
 }
 
-static uint32_t aac_afh_meta_write_cb(void *user_data, void *dest, uint32_t want)
+static ssize_t aac_afh_meta_write_cb(void *user_data, void *dest, size_t count)
 {
 	int fd = *(int *)user_data;
-	return write(fd, dest, want);
+	return write(fd, dest, count);
 }
 
 static uint32_t aac_afh_meta_truncate_cb(void *user_data)
@@ -238,11 +238,7 @@ static int aac_afh_rewrite_tags(const char *map, size_t mapsize,
 	replace_or_add_tag("album", tags->album, metadata);
 	replace_or_add_tag("date", tags->year, metadata);
 	replace_or_add_tag("comment", tags->comment, metadata);
-	ret = -E_MP4_META_WRITE;
-	if (!mp4_meta_update(mp4))
-		goto close;
-	ret = 1;
-close:
+	ret = mp4_meta_update(mp4);
 	mp4_close(mp4);
 	return ret;
 }
diff --git a/error.h b/error.h
index 671d0612..843c4ee8 100644
--- a/error.h
+++ b/error.h
@@ -141,7 +141,6 @@
 	PARA_ERROR(MP4_BAD_SAMPLE, "mp4: invalid sample number"), \
 	PARA_ERROR(MP4_BAD_SAMPLERATE, "mp4: invalid sample rate"), \
 	PARA_ERROR(MP4_BAD_SAMPLE_COUNT, "mp4: invalid number of samples"), \
-	PARA_ERROR(MP4_META_WRITE, "mp4: could not update mp4 metadata"), \
 	PARA_ERROR(MP4_OPEN, "mp4: open failed"), \
 	PARA_ERROR(MP4_TRACK, "mp4: no audio track"), \
 	PARA_ERROR(MP4_MISSING_ATOM, "mp4: essential atom not found"), \
diff --git a/mp4.c b/mp4.c
index 535c785a..fc8c41b4 100644
--- a/mp4.c
+++ b/mp4.c
@@ -970,22 +970,27 @@ static void *modify_moov(struct mp4 *f, uint32_t *out_size)
 	return out_buffer;
 }
 
-static int32_t write_data(struct mp4 *f, void *data, uint32_t size)
+static int write_data(struct mp4 *f, void *data, size_t size)
 {
-	int32_t result = 1;
-
-	result = f->cb->write(f->cb->user_data, data, size);
-
-	f->current_position += size;
-
-	return result;
+	while (size > 0) {
+		ssize_t ret = f->cb->write(f->cb->user_data, data, size);
+		if (ret < 0) {
+			if (errno == EINTR)
+				continue;
+			return -ERRNO_TO_PARA_ERROR(errno);
+		}
+		f->current_position += ret;
+		size -= ret;
+	}
+	return 1;
 }
 
-int32_t mp4_meta_update(struct mp4 *f)
+int mp4_meta_update(struct mp4 *f)
 {
 	void *new_moov_data;
 	uint32_t new_moov_size;
-	uint8_t buf[4];
+	uint8_t buf[8] = "----moov";
+	int ret;
 
 	set_position(f, 0);
 	new_moov_data = modify_moov(f, &new_moov_size);
@@ -995,17 +1000,24 @@ int32_t mp4_meta_update(struct mp4 *f)
 	}
 	if (f->last_atom != ATOM_MOOV) {
 		set_position(f, f->moov_offset + 4);
-		write_data(f, "free", 4); /* rename old moov to free */
+		ret = write_data(f, "free", 4); /* rename old moov to free */
+		if (ret < 0)
+			goto free_moov;
 		set_position(f, f->file_size); /* write new moov atom at EOF */
 	} else /* overwrite old moov atom */
 		set_position(f, f->moov_offset);
 	write_u32_be(buf, new_moov_size + 8);
-	write_data(f, buf, 4);
-	write_data(f, "moov", 4);
-	write_data(f, new_moov_data, new_moov_size);
-	free(new_moov_data);
+	ret = write_data(f, buf, sizeof(buf));
+	if (ret < 0)
+		goto free_moov;
+	ret = write_data(f, new_moov_data, new_moov_size);
+	if (ret < 0)
+		goto free_moov;
 	f->cb->truncate(f->cb->user_data);
-	return 1;
+	ret = 1;
+free_moov:
+	free(new_moov_data);
+	return ret;
 }
 
 static char *meta_find_by_name(const struct mp4 *f, const char *item)
diff --git a/mp4.h b/mp4.h
index 783de19d..30a609f3 100644
--- a/mp4.h
+++ b/mp4.h
@@ -1,9 +1,9 @@
 struct mp4_callback {
-    ssize_t (*read)(void *user_data, void *buffer, size_t length);
-    uint32_t (*write)(void *udata, void *buffer, uint32_t length);
-    uint32_t (*seek)(void *user_data, uint64_t position);
-    uint32_t (*truncate)(void *user_data);
-    void *user_data;
+	ssize_t (*read)(void *user_data, void *buffer, size_t length);
+	ssize_t (*write)(void *user_data, void *buffer, size_t count);
+	uint32_t (*seek)(void *user_data, uint64_t position);
+	uint32_t (*truncate)(void *user_data);
+	void *user_data;
 };
 
 struct mp4_tag {
@@ -29,7 +29,7 @@ int32_t mp4_num_samples(const struct mp4 *f);
 uint64_t mp4_get_duration(const struct mp4 *f);
 int mp4_open_meta(const struct mp4_callback *cb, struct mp4 **result);
 struct mp4_metadata *mp4_get_meta(struct mp4 *f);
-int32_t mp4_meta_update(struct mp4 *f);
+int mp4_meta_update(struct mp4 *f);
 char *mp4_meta_get_artist(const struct mp4 *f);
 char *mp4_meta_get_title(const struct mp4 *f);
 char *mp4_meta_get_date(const struct mp4 *f);