More memory allocation cleanup in qacp

Net removal of 12 strdup calls
rstat is properly freed
Don't need to strdup for basename, the original is not modified

1 files changed, 62 insertions, 65 deletions

diff --git a/qacp.c b/qacp.c
index 0c23f7e..2e04bd5 100644
--- a/qacp.c
+++ b/qacp.c
@@ -121,6 +121,13 @@ qacp_rstat(const char *rmtpath, int *rstaterrno)
 }
 
 void
+free_rstat(struct qp_rstat_pkt *p)
+{
+	free(p->qp_path);
+	free(p);
+}
+
+void
 qacp_sendonefile(const char *host, const char *srcfile, const char *destfile)
 {
 	struct qa_packet *qp;
@@ -275,6 +282,7 @@ qacp_recvonefile(const char *host, const char *srcfile, const char *destfile)
 			(long long int)rstatp->qp_st_size);
 	}
 	close(outfd);
+	free_rstat(rstatp);
 
 	/* Await our return code from qarshd */
 	qp = recv_packet(qacp_fd);
@@ -285,6 +293,7 @@ qacp_recvonefile(const char *host, const char *srcfile, const char *destfile)
 		close(qacp_fd);
 		exit(125);
 	}
+	if (qp) qpfree(qp);
 
 	if (!quiet) {
 		printf("%s:%s  -> %30.30s\n", host, srcfile, destfile);
@@ -307,7 +316,6 @@ recvfiles(char **argv, int argc, int fileidx, short recursive)
 	char *destpath;
 	int destisdir;
 	char *tmpstr;
-	char *ldnp;  /* local path dirname */
 	char *rbnp;  /* remote path basename */
 	struct qp_rstat_pkt *rstatp;
 	struct stat sb;
@@ -332,11 +340,12 @@ recvfiles(char **argv, int argc, int fileidx, short recursive)
 		 * if that directory exists. 
 		 */
 		tmpstr = strdup(destpath);
-		ldnp = strdup(dirname(tmpstr));
+		cp = strdup(dirname(tmpstr));
 		free(tmpstr);
-		if (stat(ldnp, &sb) < 0) {
-			fprintf(stderr, "%s: %s\n", ldnp, strerror(errno));
-			return -1;
+		if (stat(cp, &sb) < 0) {
+			fprintf(stderr, "%s: %s\n", cp, strerror(errno));
+			free(cp);
+			goto error_free_destpath;
 		}
 
 		if (S_ISDIR(sb.st_mode)) {
@@ -346,9 +355,11 @@ recvfiles(char **argv, int argc, int fileidx, short recursive)
 			 */
 			destisdir = 0;
 		} else {
-			fprintf(stderr, "%s not a directory\n", ldnp);
-			return -1;
+			fprintf(stderr, "%s not a directory\n", cp);
+			free(cp);
+			goto error_free_destpath;
 		}
+		free(cp);
 	} else {
 		if (S_ISDIR(sb.st_mode)) {
 			destisdir = 1;
@@ -356,9 +367,14 @@ recvfiles(char **argv, int argc, int fileidx, short recursive)
 			destisdir = 0;
 		} else {
 			fprintf(stderr, "%s not a file or directory\n", destpath);
-			return -1;
+			goto error_free_destpath;
 		}
 	}
+
+	if (!destisdir && argc - fileidx > 2) {
+		fprintf(stderr, "Will not copy multiple remote files to the same local file\n");
+		goto error_free_destpath;
+	}
 	
 
 	for (file = fileidx; file < (argc - 1); file++) {
@@ -367,19 +383,19 @@ recvfiles(char **argv, int argc, int fileidx, short recursive)
 	/* Rstat the file, if it is a dir, error */
 	/* Otherwise copy it to local location */
 
-		if (strchr(argv[file], ':') == NULL) {
+		if ((rmtpath = strchr(argv[file], ':')) == NULL) {
 			fprintf(stderr, "%s is not a valid remote host:file string\n",
 		        	argv[file]);
-			return -1; 
+			goto error_free_destpath;
 		}
 
-		rhost = strdup(argv[file]);
-		cp = strchr(rhost, ':');
-		*cp = '\0';
+		/* Split the remote path from the rest */
+		*(rmtpath++) = '\0';
+		rhost = argv[file];
 
 		/* Grab any user/group info */
-		if ((cp = strchr(rhost, '@'))) {
-			ruser = rhost;
+		if ((cp = strchr(argv[file], '@'))) {
+			ruser = argv[file];
 			rhost = cp+1;
 			*cp = '\0';
 		}
@@ -389,13 +405,13 @@ recvfiles(char **argv, int argc, int fileidx, short recursive)
 			*cp = '\0';
 		}
 
-		if (!(pw = getpwuid(getuid()))) {
-			fprintf(stderr, "qacp: unknown user id.\n");
-			return -1;
-		}
 
 		if (ruser == NULL) {
-			ruser = strdup(pw->pw_name);
+			if (!(pw = getpwuid(getuid()))) {
+				fprintf(stderr, "qacp: unknown user id.\n");
+				goto error_free_destpath;
+			}
+			ruser = pw->pw_name;
 		}
 
 		qacp_fd = connect_to_host(rhost, QARSHD_CONTROL_PORT, &qarsh_ss_family);
@@ -407,24 +423,16 @@ recvfiles(char **argv, int argc, int fileidx, short recursive)
 				fprintf(stderr, "Could not connect to %s:%d, %d: %s\n",
 					rhost, QARSHD_CONTROL_PORT, errno, strerror(errno));
 			}
-			return -1;
+			goto error_free_destpath;
 		}
 
 		set_remote_user(ruser, rgrp);
 
-		cp = strdup(argv[file]);
-		cp = strchr(argv[file], ':');
-		cp++;
-
-		rmtpath = strdup(cp);
-
 		rstatp = qacp_rstat(rmtpath, &rstaterrno);
 		if (rstatp) {
 			if (S_ISREG(rstatp->qp_st_mode)) {
 				if (destisdir) {
-					cp = strdup(rmtpath);
-					rbnp = strdup(basename(cp));
-					free(cp);
+					rbnp = basename(rmtpath);
 
 					cp = malloc(strlen(destpath) + strlen(rbnp) + 2);
 					strcpy(cp, destpath);
@@ -440,24 +448,33 @@ recvfiles(char **argv, int argc, int fileidx, short recursive)
 			}
 			if (S_ISDIR(rstatp->qp_st_mode)) {
 				fprintf(stderr, "%s: Not a regular file\n", argv[file]);
+				free(destpath);
+				free(rmtpath);
+				free_rstat(rstatp);
 				return -1;
 			}
+			free_rstat(rstatp);
 		} else {
 			fprintf(stderr, "%s: %s\n", argv[file], strerror(rstaterrno));
-			return -1;
+			goto error_free_destpath;
 		}
 
 		close(qacp_fd);
-	}
+	} /* for file */
+	free(destpath);
 
 	return 0;
+
+error_free_destpath:
+	free(destpath);
+	return -1;
 }
 
 
 int
 sendfiles(char **argv, int argc, int fileidx, short recursive)
 {
-	char *cp, *cp2, *freeme;
+	char *cp;
 	char *rhost = NULL;
 	char *ruser = NULL;
 	char *rgrp = NULL;
@@ -472,19 +489,18 @@ sendfiles(char **argv, int argc, int fileidx, short recursive)
 	int rstaterrno;
 
 
-	if (strchr(argv[argc-1], ':') == NULL) {
+	if ((rmtpath = strchr(argv[argc-1], ':')) == NULL) {
 		fprintf(stderr, "%s is not a valid remote host:file string\n",
 		        argv[argc-1]);
 		return -1; 
 	}
 
-	freeme = rhost = strdup(argv[argc-1]);
-	cp = strchr(rhost, ':');
-	*cp = '\0';
+	*(rmtpath++) = '\0';
+	rhost = argv[argc-1];
 
 	/* Grab any user/group info */
 	if ((cp = strchr(rhost, '@'))) {
-		ruser = rhost;
+		ruser = argv[argc-1];
 		rhost = cp+1;
 		*cp = '\0';
 	}
@@ -494,14 +510,12 @@ sendfiles(char **argv, int argc, int fileidx, short recursive)
 		*cp = '\0';
 	}
 
-	if (!(pw = getpwuid(getuid()))) {
-		fprintf(stderr, "qacp: unknown user id.\n");
-		free(freeme);
-		return -1;
-	}
-
 	if (ruser == NULL) {
-		ruser = strdup(pw->pw_name);
+		if (!(pw = getpwuid(getuid()))) {
+			fprintf(stderr, "qacp: unknown user id.\n");
+			return -1;
+		}
+		ruser = pw->pw_name;
 	}
 
 	qacp_fd = connect_to_host(rhost, QARSHD_CONTROL_PORT, &qarsh_ss_family);
@@ -514,24 +528,15 @@ sendfiles(char **argv, int argc, int fileidx, short recursive)
 				rhost, QARSHD_CONTROL_PORT, errno, strerror(errno));
 		}
 
-		free(freeme);
 		return -1;
 	}
 
 	set_remote_user(ruser, rgrp);
 
-	cp = strdup(argv[argc-1]);
-	cp2 = strchr(cp, ':'); 
-	cp2++;
-	rmtpath = strdup(cp2);
-	free(cp);
-
 	rstatp = qacp_rstat(rmtpath, &rstaterrno);
 
 	for (file = fileidx; file < (argc - 1); file++) {
-		tmpstr = strdup(argv[file]);
-		lbnp = strdup(basename(tmpstr));
-		free(tmpstr);
+		lbnp = basename(argv[file]);
 
 		if (rstatp) {
 			/* 
@@ -566,31 +571,23 @@ sendfiles(char **argv, int argc, int fileidx, short recursive)
 			 	* that /tmp exists, we leave destpath == rmtpath.
 				*/
 				destpath = strdup(rmtpath);
-				free(tmprstatp->qp_path);
-				free(tmprstatp);
+				free_rstat(tmprstatp);
 			} else {
 				fprintf(stderr, "%s:%s - %s\n", rhost, rmtpath, 
 				        strerror(rstaterrno));
-				free(freeme);
 				return -1;
 			}
 		}
 
 		qacp_sendonefile(rhost, argv[file], destpath);
 
-		free(lbnp);
 		free(destpath);
 	}
 
 	if (rstatp) {
-		free(rstatp->qp_path);
-		free(rstatp);
+		free_rstat(rstatp);
 	}
 
-	free(freeme);
-	free(ruser);
-	free(rmtpath);
-
 	return 0;
 }