[prev in list] [next in list] [prev in thread] [next in thread] 

List:       coreutils-bug
Subject:    coreutils-5.0: bugs in readlink --canonicalize
From:       Jim Meyering <jim () meyering ! net>
Date:       2003-04-10 17:22:45
Message-ID: 85n0iygtcq.fsf () pi ! meyering ! net
[Download RAW message or body]

`readlink --canonicalize' (aka lib/canonicalize.c) had problems
on e.g., Solaris systems.  This affects only systems that lack
canonicalize_file_name and that have the resolvepath function,
like Solaris.  Here are examples of how it fails:

  $ readlink --canonical .
  [Exit 1]
  $ readlink --canonical /no-such-file
  readlink: memory exhausted
  [Exit 1]
  $ readlink --canonical /
  /cÈ

Note that in the last example above, there are two extra bytes
after the `/'.

Although df.c also uses canonicalize_file_name, it's not affected.

Here's the fix:

	* canonicalize.c (canonicalize_file_name) [HAVE_RESOLVEPATH]: Remove
	stray semicolon that caused `readlink --canonical RELATIVE_NAME' to
	fail on e.g., Solaris systems.  Reported by Bruce Korb.
	(canonicalize_file_name): Return NULL immediately if resolvepath fails.
	Otherwise, `readlink --canonical /no-such-file' would exhaust
	virtual memory on some systems (e.g. Solaris).
	(canonicalize_file_name): Always free `extra_buf' before returning.
	(canonicalize_file_name): NUL-terminate the result.

Yes, I'll be adding a few tests, soon :-)

Index: lib/canonicalize.c
===================================================================
RCS file: /fetish/cu/lib/canonicalize.c,v
retrieving revision 1.3
retrieving revision 1.7
diff -u -p -u -p -r1.3 -r1.7
--- lib/canonicalize.c	20 Jan 2003 16:41:19 -0000	1.3
+++ lib/canonicalize.c	10 Apr 2003 17:03:26 -0000	1.7
@@ -130,7 +130,7 @@ canonicalize_file_name (const char *name
     {
       char *wd;
 
-      if (!(wd = xgetcwd ()));
+      if (!(wd = xgetcwd ()))
 	return NULL;
 
       extra_buf = path_concat (wd, name, NULL);
@@ -147,18 +147,22 @@ canonicalize_file_name (const char *name
       resolved_size = 2 * resolved_size + 1;
       resolved = xmalloc (resolved_size);
       resolved_len = resolvepath (name, resolved, resolved_size);
+      if (resolved_len < 0)
+	{
+	  free (resolved);
+	  free (extra_buf);
+	  return NULL;
+	}
       if (resolved_len < resolved_size)
 	break;
       free (resolved);
     }
 
-  if (resolved_len < 0)
-    {
-      free (resolved);
-      resolved = NULL;
-    }
-
   free (extra_buf);
+
+  /* NUL-terminate the resulting name.  */
+  resolved[resolved_len] = '\0';
+
   return resolved;
 
 #else /* !HAVE_RESOLVEPATH */



[prev in list] [next in list] [prev in thread] [next in thread]