Mailing-List: contact cygwin-apps-help AT cygwin DOT com; run by ezmlm
Sender: cygwin-apps-owner AT cygwin DOT com
List-Subscribe: <mailto:cygwin-apps-subscribe AT cygwin DOT com>
List-Archive: <http://sources.redhat.com/ml/cygwin-apps/>
List-Post: <mailto:cygwin-apps AT cygwin DOT com>
List-Help: <mailto:cygwin-apps-help AT cygwin DOT com>, <http://sources.redhat.com/lists.html#faqs>
Mail-Followup-To: cygwin-apps AT cygwin DOT com
Delivered-To: mailing list cygwin-apps AT cygwin DOT com
Date: Tue, 14 May 2002 00:57:26 -0400
From: Christopher Faylor <cgf AT redhat DOT com>
To: cygwin-apps AT cygwin DOT com
Subject: Re: Setup 2.218.2.4 (fixed?)
Message-ID: <20020514045726.GA23058@redhat.com>
Reply-To: cygwin-apps AT cygwin DOT com
Mail-Followup-To: cygwin-apps AT cygwin DOT com
References: <FC169E059D1A0442A04C40F86D9BA7600C6082 AT itdomain003 DOT itdomain DOT net DOT au> <20020513161941 DOT GA814 AT redhat DOT com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20020513161941.GA814@redhat.com>
User-Agent: Mutt/1.3.23.1i

On Mon, May 13, 2002 at 12:19:41PM -0400, Christopher Faylor wrote:
>On Mon, May 13, 2002 at 09:05:52PM +1000, Robert Collins wrote:
>>>I've noticed the same problem on a non-smb directory, fwiw.
>>
>>dir you mean as in "d:\foo" or as in NFS mounted?
>
>Occam's razor.

I think I've tracked this problem down.  I've checked in a couple of
fixes in the branch.  I've put up a new version of setup.exe on
sourceware.

I believe that there were a couple of problems.  One was that setup.exe
would consider anything with the string "setup.ini" to be a potential
setup.ini file.  The reason for this is that Robert (or someone) seems
enamored of the use of strstr for path manipulation.  I think I've
mentioned before that you really can't do a strstr (path, "foo.bar") and
assume that any foo.bar that means that the file component is "foo.bar".
This assumption exists in several places in the source code.

In my case, setup.exe was parsing files like setup.ini.old, setup.ini~,
and setup.ini.orig.  This caused problems since the assumption was that
the setup.ini was the trailing component of a path spec, which it
wasn't.

The other problem was the clobbering of a global buffer.  This resulted
in either a stack overrun or just a hung setup.exe.

I've attached the patch and ChangeLog below.  I didn't try to check this
into the trunk since I thought that Robert would look at it and want to
do something another way.

Can I suggest that probably setup.exe needs something like a "suffix"
function which operates similarly to strstr but only returns a non-NULL
pointer if the string matches the last strlen(string) characters of
a path?

cgf

2002-05-14  Christopher Faylor  <cgf AT redhat DOT com>

        * find.cc (find_sub): Be more defensive in preserving trailing parts of
        components when doing recursive directory searches or calling user
        supplied for_each().
        * ini.cc (find_routine): Don't assume that any path name with
        "setup.ini" in it is actually a setup.ini file.  Only honor trailing
        components.  Copy path argument to temporary storage when unescaping
        to prevent nuking of argument.

Index: find.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/find.cc,v
retrieving revision 2.3
retrieving revision 2.3.6.1
diff -u -r2.3 -r2.3.6.1
--- find.cc	18 Feb 2002 12:35:20 -0000	2.3
+++ find.cc	14 May 2002 04:32:44 -0000	2.3.6.1
@@ -40,8 +40,7 @@
   char *end = dir + strlen (dir);
   int rv = 0;
 
-  *end++ = '/';
-  strcpy (end, "*");
+  strcpy (end, "\\*");
 
   h = FindFirstFile (dir, &wfd);
 
@@ -54,7 +53,8 @@
 	  || strcmp (wfd.cFileName, "..") == 0)
 	continue;
 
-      strcpy (end, wfd.cFileName);
+      *end = '\\';
+      strcpy (end + 1, wfd.cFileName);
 
       if (wfd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
 	find_sub (for_each);
Index: ini.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/ini.cc,v
retrieving revision 2.22
retrieving revision 2.22.2.1
diff -u -r2.22 -r2.22.2.1
--- ini.cc	1 May 2002 11:13:16 -0000	2.22
+++ ini.cc	14 May 2002 04:32:45 -0000	2.22.2.1
@@ -65,8 +65,14 @@
 static void
 find_routine (char *path, unsigned int fsize)
 {
-  if (!strstr (path, "setup.ini") )
+  /* See if the path ends in a trailing setup.ini component.
+     Just return if it doesn't. */
+  unsigned pathlen = strlen (path);
+  unsigned pathprefix_len = pathlen - 10;
+  if (pathlen < strlen ("setup.ini")
+      || strcasecmp (path + pathprefix_len, "\\setup.ini") != 0)
     return;
+
   io_stream *ini_file = io_stream::open (String ("file://") + local_dir + "/" +
 					 path, "rb");
   if (!ini_file)
@@ -82,9 +88,12 @@
   setup_timestamp = 0;
   setup_version = 0;
 
-  /* Attempt to unescape the string */
-  path[strlen(path) -10] = '\0';
-  String mirror = rfc1738_unescape_part (path);
+  /* Copy leading part of path to temporary buffer and unescape it */
+
+  char path_prefix[pathprefix_len + 1];
+  strncpy (path_prefix, path, pathprefix_len);
+  path_prefix[pathprefix_len] = '\0';
+  String mirror = rfc1738_unescape_part (path_prefix);
   ini_init (ini_file, mirror);
 
   /*yydebug = 1; */