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

user/2388: Misc fixes to mg's fileio.c (memory leaks, etc)



>Number:         2388
>Category:       user
>Synopsis:       couple of fixes to fileio.c
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    bugs
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Mon Feb 11 10:50:01 MST 2002
>Last-Modified:
>Originator:     Vincent Labrecque
>Organization:
Never been able to get that
>Release:        OpenBSD 3.0-current
>Environment:
	
	System      : OpenBSD 3.0
	Architecture: OpenBSD.i386
	Machine     : i386
>Description:
	Couple of fixes to mg fileio.c.  Removed unsafe strcpy calls, add 
  more precise error messages, and simplify some parts of the code. (there is
  more work to be done in there, this is just a quick first pass)

        Oh, and there were a couple of memory leaks in fbackup that were fixed.

>How-To-Repeat:
	mg /usr/src/usr.bin/mg/fileio.c

>Fix:
	Here's a diff :

--- /sources/src/usr.bin/mg/fileio.c	Fri Sep 21 10:08:16 2001
+++ fileio.c	Sun Feb 10 12:26:56 2002
@@ -10,6 +10,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/dir.h>
+#include <string.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <unistd.h>
@@ -47,7 +48,7 @@
 {
 
 	if ((ffp = fopen(fn, "w")) == NULL) {
-		ewprintf("Cannot open file for writing");
+		ewprintf("Cannot open file for writing : %s", strerror(errno));
 		return (FIOERR);
 	}
 
@@ -159,32 +160,31 @@
 	char  *fn;
 {
 	struct stat	sb;
-	int		from, to, serrno;
+	int		from, to, saved;
 	size_t		nread;
-	size_t		len;
 	char		buf[BUFSIZ];
 	char		*nname;
 
-	len = strlen(fn);
-	if ((nname = malloc(len + 1 + 1)) == NULL) {
-		ewprintf("Can't get %d bytes", len + 1 + 1);
+	if (asprintf(&nname, "%s~", fn) == -1) {
+		ewprintf("Can't allocate temp file name : %s",
+		    strerror(errno));
 		return (ABORT);
 	}
-	(void) strcpy(nname, fn);
-	(void) strcpy(nname + len, "~");
-
 	if (stat(fn, &sb) == -1) {
-		ewprintf("Can't stat %s", fn);
+		ewprintf("Can't stat %s : %s", fn, strerror(errno));
 		return (FALSE);
 	}
 
-	if ((from = open(fn, O_RDONLY)) == -1)
+	if ((from = open(fn, O_RDONLY)) == -1) {
+		free(nname);
 		return (FALSE);
+	}
 	to = open(nname, O_WRONLY|O_CREAT|O_TRUNC, (sb.st_mode & 0777));
 	if (to == -1) {
-		serrno = errno;
+		saved = errno;
 		close(from);
-		errno = serrno;
+		free(nname);
+		errno = saved;
 		return (FALSE);
 	}
 	while ((nread = read(from, buf, sizeof(buf))) > 0) {
@@ -193,13 +193,16 @@
 		    break;
 		}
 	}
-	serrno = errno;
+	saved = errno;
 	close(from);
 	close(to);
-	if (nread == -1)
-		unlink(nname);
+	if (nread == -1) {
+		if (unlink(nname) == -1)
+			ewprintf("Can't unlink temp : %s", strerror(errno));
+	}
 	free(nname);
-	errno = serrno;
+	errno = saved;
+	
 	return (nread == -1 ? FALSE : TRUE);
 }
 #endif
@@ -310,8 +313,8 @@
 				cp[-1] = '/';
 #endif
 				--cp;
-				while (cp > fnb && *--cp != '/') {
-				}
+				while (cp > fnb && *--cp != '/')
+					;
 				++cp;
 				if (fn[2] == '\0') {
 					*--cp = '\0';
@@ -453,7 +456,7 @@
 		return NULL;
 	}
 	bp->b_dotp = lforw(bp->b_linep);	/* go to first line */
-	(void) strncpy(bp->b_fname, dirname, NFILEN);
+	(void) strlcpy(bp->b_fname, dirname, sizeof bp->b_fname);
 	if ((bp->b_modes[0] = name_mode("dired")) == NULL) {
 		bp->b_modes[0] = name_mode("fundamental");
 		ewprintf("Could not find mode dired");
@@ -539,7 +542,7 @@
 		}
 	}
 	/* Now we get the prefix of the name the user typed. */
-	strcpy(prefixx, buf);
+	strlcpy(prefixx, buf, sizeof prefixx);
 	cp = strrchr(prefixx, '/');
 	if (cp == NULL)
 		prefixx[0] = 0;


>Audit-Trail:
>Unformatted: