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

Re: [MiNT] ext2 bug Was Re: another getcwd fix (includes previous patch)



On Wed, 2008-01-09 at 20:44 +0100, Vincent Rivière wrote:
> Alan Hourihane a wrote:
> > I've also attached a new getcwd.c (& unx2dos.c) fix that adds __set_errno(ENOMEM)
> > on out of memory conditions to ensure we return errno's correctly.
> 
> Hello, Alan !
> I'm back to the MiNTLib, and I'm reviewing the pending patches.
> 
> Just a remark about __getcwd():
> 
> 	if (_rootdir && drv == _rootdir) {
> 		if (!*path) {
> 			path[0] = '\\';
> 			path[1] = '\0';
> 		}
> 		_dos2unx(path, buf, size);
> 	}
> 	else
> 		/* convert DOS filename to unix */
> 		_dos2unx(_path, buf, size);
> 
> 	free(_path);
> 
> 	if (errno == ENAMETOOLONG) {
> 		if (buf_malloced)
> 			free(buf);
> 		return NULL;
> 	}
> 
> The errno handling is wrong.
> 1) The value of errno is relevant only when a function has failed. But 
> you didn't check for the return value of _dos2unx()
> 2) The value of errno may be overwritten when functions are called, such 
> as free(). You should check errno just after calling _dos2unx(), but 
> only when it fails.

Ack, yes, Fixed. Thanks for the review Vincent!

Alan.
Index: mintlib/getcwd.c
===================================================================
RCS file: /mint/mintlib/mintlib/getcwd.c,v
retrieving revision 1.6
diff -u -r1.6 getcwd.c
--- mintlib/getcwd.c	8 Oct 2003 15:23:14 -0000	1.6
+++ mintlib/getcwd.c	10 Jan 2008 16:20:47 -0000
@@ -24,20 +24,29 @@
 char *
 __getcwd (char *buf, size_t size)
 {
-	const int len = (size > 0 ? size : PATH_MAX) + 16;
-	char _path[len]; /* XXX non ANSI */
-	char *path;
+	int len;
+	char *path, *_path;
 	char drv;
 	int buf_malloced = 0;
 	int r;
 
+	len = (size > 0 ? size : PATH_MAX) + 16;
+	_path = malloc(len);
+	if (!_path) {
+		__set_errno(ENOMEM);
+		return NULL;
+	}
+
 	if (!buf) {
 		if (size == 0)
 			size = PATH_MAX;
 		
 		buf = malloc (size);
-		if (!buf)
+		if (!buf) {
+			free(_path);
+			__set_errno(ENOMEM);
 			return NULL;
+		}
 		
 		buf_malloced = 1;
 	}
@@ -50,6 +59,7 @@
 
 	r = (int) Dgetcwd(path, 0, len - 2);
 	if (r != 0 && r != -ENOSYS) {
+		free(_path);
 		if (buf_malloced)
 			free(buf);
 		__set_errno (-r);
@@ -64,12 +74,23 @@
 			path[0] = '\\';
 			path[1] = '\0';
 		}
-		_dos2unx(path, buf, size);
+		r = _dos2unx(path, buf, size);
 	}
-	else
+	else {
 		/* convert DOS filename to unix */
-		_dos2unx(_path, buf, size);
+		r = _dos2unx(_path, buf, size);
+	}
+
+	/* dos2unx failed, probably ENAMETOOLONG, so abort now */
+	if (r == -1) {
+		free(_path);
+		if (buf_malloced)
+			free(buf);
+		return NULL;
+	}
 	
+	free(_path);
+
 	if (buf_malloced) {
 		size_t len = strlen (buf) + 1;
 		void *newptr;
Index: mintlib/unx2dos.c
===================================================================
RCS file: /mint/mintlib/mintlib/unx2dos.c,v
retrieving revision 1.5
diff -u -r1.5 unx2dos.c
--- mintlib/unx2dos.c	8 Oct 2003 15:23:14 -0000	1.5
+++ mintlib/unx2dos.c	10 Jan 2008 16:20:47 -0000
@@ -1,5 +1,6 @@
 
 #include <ctype.h>
+#include <errno.h>
 #include <limits.h>
 #include <stdio.h>
 #include <string.h>
@@ -11,11 +12,15 @@
 
 /*
  * returns 0 for ordinary files, 1 for special files (like /dev/tty)
+ *
+ * or returns -1 on error, such as ENAMETOOLONG
  */
 
 int
 _unx2dos(const char *unx, char *dos, size_t len)
 {
+	register int unx_length = strlen(unx);
+	register int count = 0;
 	const char *u;
 	char *d, c;
 
@@ -76,6 +81,7 @@
 	}
 
 	while( (c = *u++) != 0 ) {
+		count++;
 		if (c == '/')
 			c = '\\';
 #if 0
@@ -91,8 +97,14 @@
 #endif
 		*d++ = c;
 		len--;
-		if (len == 0)
+		if (len == 0) {
+			if (count < unx_length) {
+				__set_errno(ENAMETOOLONG);
+				*d = 0;
+				return -1;
+			}
 			break;
+		}
 	}
 	*d = 0;
 	return 0;
@@ -101,6 +113,8 @@
 int
 _dos2unx(const char *dos, char *unx, size_t len)
 {
+	register int dos_length = strlen(dos);
+	register int count = 0;
 	register char c;
 
 	len--;			/* for terminating NUL */
@@ -133,14 +147,21 @@
 	/* convert slashes
 	 */
 	while ( (c = *dos++) != 0) {
+		count++;
 		if (c == '\\')
 			c = '/';
 		else if (__mint < 7)
 			c = tolower(c);
 		*unx++ = c;
 		len--;
-		if (len == 0)
+		if (len == 0) {
+			if (count < dos_length) {
+				__set_errno(ENAMETOOLONG);
+				*unx = 0;
+				return -1;
+			}
 			break;
+		}
 	}
 	*unx = 0;
 	return 0;