[PATCH 5/5] ptp-gadget: Fix problems with image upload

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This patch fixes several bugs in upload functionality:
Upload didn't work on vfat file system as the reservation
algorithm using hard links can not work on vfat file system.
Reservation algorithm is changed now and doesn't use
hard links any more.

Further testing with older host software versions (e.g. on
Ubuntu 8.04) showed following issue: if the host didn't request
storage info while connecting, upload can't function as the
free space counter remains uninitialized indicating no space.

Testing upload with big files (> 5MB) showed another issue:
upload of big image files didn't work. Fix this by receiving
image data using smaller data chuncks.

Signed-off-by: Anatolij Gustschin <agust@xxxxxxx>
---
 ptp.c |  162 +++++++++++++++++++++++++++++++++++++++++++++++++----------------
 1 files changed, 123 insertions(+), 39 deletions(-)

diff --git a/ptp.c b/ptp.c
index 60d8fe5..0a8bdf0 100644
--- a/ptp.c
+++ b/ptp.c
@@ -1565,10 +1565,9 @@ static int update_free_space(void)
 		fprintf(stderr, "statfs %s: %s\n", root, strerror(errno));
 		return ret;
 	}
-
 	if (verbose > 1)
-		fprintf(stdout, "Block-size %d, total 0x%lx, free 0x%lx\n",
-			fs.f_bsize, fs.f_blocks, fs.f_bfree);
+		fprintf(stdout, "Block-size %d, total %d, free %d\n",
+			fs.f_bsize, (int)fs.f_blocks, (int)fs.f_bfree);
 
 	bytes = (unsigned long long)fs.f_bsize * fs.f_bfree;
 	storage_info.free_space_in_bytes = __cpu_to_le64(bytes);
@@ -1708,8 +1707,9 @@ static int process_send_object_info(void *recv_buf, void *send_buf)
 	char lock_file[256];
 	char new_file[256];
 	mode_t mode;
-	int fd;
-	int ret = 0;
+	int fd, fd_new;
+	int ret = 0, len;
+	char fs_buf[32];
 
 	param = (uint32_t *)r_container->payload;
 	p1 = __le32_to_cpu(*param);
@@ -1760,10 +1760,14 @@ static int process_send_object_info(void *recv_buf, void *send_buf)
 		break;
 	}
 
-	if (info->object_compressed_size > storage_info.free_space_in_bytes) {
+	if (((uint64_t)__le32_to_cpu(info->object_compressed_size)) >
+	    __le64_to_cpu(storage_info.free_space_in_bytes)) {
 		code = PIMA15740_RESP_STORE_FULL;
-		if (verbose)
-			fprintf(stdout, "no space\n");
+		if (verbose) {
+			fprintf(stdout, "no space: free %lld, req. %d\n",
+				storage_info.free_space_in_bytes,
+				info->object_compressed_size);
+		}
 		goto resp;
 	}
 
@@ -1840,9 +1844,9 @@ static int process_send_object_info(void *recv_buf, void *send_buf)
 		goto err;
 	}
 
-	ret = link(lock_file, new_file);
-	if (ret < 0) {
-		fprintf(stderr, "link: old %s, new %s: %s\n",
+	fd_new = open(new_file, O_CREAT | O_EXCL | O_WRONLY, mode);
+	if (fd_new < 0) {
+		fprintf(stderr, "open: lock file %s, new file %s: %s\n",
 			lock_file, new_file, strerror(errno));
 		if (errno == EEXIST)
 			code = PIMA15740_RESP_STORE_NOT_AVAILABLE;
@@ -1857,24 +1861,27 @@ static int process_send_object_info(void *recv_buf, void *send_buf)
 		goto err;
 	}
 
-	ret = ftruncate(fd, info->object_compressed_size);
+	len = snprintf(fs_buf, sizeof(fs_buf), "%d",
+		       info->object_compressed_size);
+	ret = ftruncate(fd, len);
 	if (ret < 0) {
-		fprintf(stderr, "ftruncate: %s: %s\n",
+		fprintf(stderr, "ftruncate for lock file: %s: %s\n",
 			lock_file, strerror(errno));
-		code = PIMA15740_RESP_STORE_FULL;
-		close(fd);
-
-		ret = unlink(new_file);
-		if (ret < 0)
-			fprintf(stderr, "can't remove %s: %s\n",
-				new_file, strerror(errno));
+		goto err_del;
+	}
 
-		ret = unlink(lock_file);
-		if (ret < 0)
-			fprintf(stderr, "can't remove %s: %s\n",
-				lock_file, strerror(errno));
+	ret = write(fd, fs_buf, len);
+	if (ret < 0) {
+		fprintf(stderr, "write new file size to %s: %s\n",
+			lock_file, strerror(errno));
+		goto err_del;
+	}
 
-		goto err;
+	ret = ftruncate(fd_new, info->object_compressed_size);
+	if (ret < 0) {
+		fprintf(stderr, "ftruncate: %s: %s\n",
+			new_file, strerror(errno));
+		goto err_del;
 	}
 
 	memcpy(&object_info_p->info, info, new_info_size);
@@ -1893,9 +1900,25 @@ static int process_send_object_info(void *recv_buf, void *send_buf)
 	param[2] = __cpu_to_le32(last_object_number);
 
 	close(fd);
+	close(fd_new);
 resp:
 	make_response(s_container, r_container, code, sizeof(*s_container) + 12);
 	return 0;
+
+err_del:
+	code = PIMA15740_RESP_STORE_FULL;
+	close(fd);
+	close(fd_new);
+
+	ret = unlink(new_file);
+	if (ret < 0)
+		fprintf(stderr, "can't remove %s: %s\n",
+			new_file, strerror(errno));
+
+	ret = unlink(lock_file);
+	if (ret < 0)
+		fprintf(stderr, "can't remove %s: %s\n",
+			lock_file, strerror(errno));
 err:
 	free(object_info_p);
 	object_info_p = 0;
@@ -1992,19 +2015,29 @@ static int process_send_object(void *recv_buf, void *send_buf)
 
 	/* more data? */
 	if (obj_size > cnt) {
+		unsigned int rest, recv_len;
+		void *data = map + cnt;
+
 		if (verbose) {
 			fprintf(stderr, "Reading rest %d of %d\n",
 				obj_size - cnt, obj_size);
 		}
-		ret = bulk_read(map + cnt, obj_size - cnt);
-		if (ret < 0) {
-			fprintf(stderr, "%s: reading data for %s failed: %s\n",
-				__func__, object_info_p->name, strerror(errno));
-			code = PIMA15740_RESP_INCOMPLETE_TRANSFER;
-			munmap(map, obj_size);
-			close(fd);
-			errno = EPIPE;
-			return ret;
+		rest = obj_size - cnt;
+		recv_len = 8192;
+		while (rest) {
+			cnt = min(rest, recv_len);
+			ret = bulk_read(data, cnt);
+			if (ret < 0) {
+				fprintf(stderr, "%s: reading data for %s failed: %s\n",
+					__func__, object_info_p->name, strerror(errno));
+				code = PIMA15740_RESP_INCOMPLETE_TRANSFER;
+				munmap(map, obj_size);
+				close(fd);
+				errno = EPIPE;
+				return ret;
+			}
+			rest -= ret;
+			data += ret;
 		}
 	}
 
@@ -2724,7 +2757,9 @@ static void clean_up(const char *path)
 	struct dirent *dentry;
 	DIR *d;
 	char file_name[256];
-	int ret;
+	int fd, ret;
+	char *endptr;
+	char fs_buf[32];
 
 	ret = chdir(path);
 	if (ret < 0)
@@ -2735,7 +2770,7 @@ static void clean_up(const char *path)
 	while ((dentry = readdir(d))) {
 		struct stat fstat;
 		char *dot;
-		int lsize, fsize;
+		unsigned long lsize, fsize;
 
 		dot = strrchr(dentry->d_name, '.');
 
@@ -2749,17 +2784,50 @@ static void clean_up(const char *path)
 		snprintf(file_name, sizeof(file_name), "%s", dentry->d_name);
 		*dot = '.';
 
-		ret = stat(dentry->d_name, &fstat);
+		fd = open(dentry->d_name, O_RDONLY);
+		if (fd < 0) {
+			fprintf(stderr, "%s: open %s: %s\n",
+				__func__, dentry->d_name, strerror(errno));
+			continue;
+		}
+
+		memset(fs_buf, 0, sizeof(fs_buf));
+		ret = read(fd, fs_buf, sizeof(fs_buf));
 		if (ret < 0) {
-			fprintf(stderr, "%s: stat %s: %s\n",
+			fprintf(stderr, "%s: read %s: %s\n",
 				__func__, dentry->d_name, strerror(errno));
+			close(fd);
 			continue;
 		}
+		close(fd);
 
-		lsize = fstat.st_size;
+		if (ret) {
+			lsize = strtoul(fs_buf, &endptr, 10);
+			fprintf(stdout, "%s: *endptr %x\n", __func__, *endptr);
+			/*
+			 * if not entire string is valid, then this file was not
+			 * created by ptp, so skip it.
+			 */
+			if (fs_buf == endptr) {
+				fprintf(stderr, "%s: can't get size of locked "
+					"file %s\n", __func__, dentry->d_name);
+				continue;
+			}
+			if (*endptr != 0)
+				continue;
+		} else {
+			/* delete empty lock */
+			ret = unlink(dentry->d_name);
+			if (ret < 0)
+				fprintf(stderr, "%s: %s: %s\n",
+					__func__, dentry->d_name,
+					strerror(errno));
+			continue;
+		}
 
 		ret = stat(file_name, &fstat);
 		if (ret < 0) {
+			/* no corresponding object file, so delete lock file */
 			fprintf(stderr, "%s: stat %s: %s\n",
 				__func__, file_name, strerror(errno));
 			ret = unlink(dentry->d_name);
@@ -2772,6 +2840,11 @@ static void clean_up(const char *path)
 
 		fsize = fstat.st_size;
 		if (lsize == fsize) {
+			/* locked file size is the same as the reserved size,
+			 * but lock file was not deleted. This means transaction
+			 * was not completed, so delete both, lock file and
+			 * appropriate object file.
+			 */
 			if (verbose)
 				printf("remove %s, %s\n",
 					dentry->d_name, file_name);
@@ -2784,6 +2857,7 @@ static void clean_up(const char *path)
 				fprintf(stderr, "%s: %s: %s\n",
 					__func__, file_name, strerror(errno));
 		} else {
+			/* no corresponding object file, so delete lock file */
 			if (verbose)
 				printf("remove %s\n", dentry->d_name);
 			ret = unlink(dentry->d_name);
@@ -2792,6 +2866,7 @@ static void clean_up(const char *path)
 					__func__, dentry->d_name, strerror(errno));
 		}
 	}
+	closedir(d);
 }
 
 static int enum_objects(const char *path)
@@ -3020,6 +3095,15 @@ int main(int argc, char *argv[])
 
 	clean_up(root);
 
+	/*
+	 * if a client doesn't ask for storage info (as seen with some
+	 * older SW versions, e.g. on Ubuntu 8.04), then the free space
+	 * in storage_info will not be updated. This might result in non
+	 * working upload because before upload the free space will be
+	 * checked. Prevent this by running update_free_space() early.
+	 */
+	update_free_space();
+
 	enum_objects(root);
 
 	if (chdir("/dev/gadget") < 0) {
-- 
1.5.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux