[patch] reorganize utils/Makefile.in

2008-03-08 Thread Brian Dessent

This patch is a revamping of the Makefile in winsup/utils.  The current
Makefile.in is fugly, in my humble opinion.  It's got lots of repeated
rules and it's not very clear how one is supposed to add or change
things.  This patch does use GNU make specific features, but I'm quite
sure we already use them in other places, e.g. $(wildcard ...),
$(patsubst ...), $(shell ...) etc. are all GNU make features AFAIK and
those are all over the place in winsup.

I've also attached a copy of the patched Makefile.in if it's easier to
review that way; the diff is quite ugly.  As you can see the total
number of lines in the file is decreased by about 70, and that's
including a number of comments that I have added to document how to use
the file.

I have tried fairly hard to make sure that no actual behavior has
changed.  I diffed a before and after run and the only real difference
seemed to be the order that things ran, as well as a couple of flags
moved to a different spot in their commands.  I also tested the case
where libbfd.a/libintl.a are absent, causing the dumper parts to be
skipped as well as missing a MinGW zlib for cygcheck.  I haven't tested
a crosscompile but I can if that's necessary.  I have tested builds in
both a combined tree as well as just winsup.

Brian2008-03-08  Brian Dessent  [EMAIL PROTECTED]

* Makefile.in: Reorganize considerably, using GNU make's
static pattern rules and target-specific variables.

 Makefile.in |  229 +---
 1 file changed, 81 insertions(+), 148 deletions(-)

Index: Makefile.in
===
RCS file: /cvs/src/src/winsup/utils/Makefile.in,v
retrieving revision 1.68
diff -u -p -r1.68 Makefile.in
--- Makefile.in 21 Dec 2007 03:32:46 -  1.68
+++ Makefile.in 8 Mar 2008 13:57:57 -
@@ -1,6 +1,6 @@
 # Makefile for Cygwin utilities
 # Copyright 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004,
-# 2005, 2006, 2007 Red Hat, Inc.
+# 2005, 2006, 2007, 2008 Red Hat, Inc.
 
 # This file is part of Cygwin.
 
@@ -36,161 +36,115 @@ override CXXFLAGS+=-fno-exceptions -fno-
 
 include $(srcdir)/../Makefile.common
 
-LIBICONV:[EMAIL PROTECTED]@
-libbfd:=${shell $(CC) -B$(bupdir2)/bfd/ --print-file-name=libbfd.a}
-libintl:=${shell $(CC) -B$(bupdir2)/intl/ --print-file-name=libintl.a}
-build_dumper:=${shell test -r $(libbfd) -a -r $(libintl) -a -n $(LIBICONV) 
 echo 1}
-
-libz:=${shell x=$$($(CC) -mno-cygwin --print-file-name=libz.a); cd $$(dirname 
$$x); dir=$$(pwd); case $$dir in *mingw*) echo $$dir/libz.a ;; esac}
-zlib_h:=-include ${patsubst %/lib/mingw/libz.a,%/include/zlib.h,${patsubst 
%/lib/libz.a,%/include/zlib.h,$(libz)}}
-zconf_h:=${patsubst %/zlib.h,%/zconf.h,$(zlib_h)}
-ifeq ${libz} 
-zlib_h:=
-zconf_h:=
-libz:=
-endif
-
-DUMPER_INCLUDES:=-I$(bupdir2)/bfd -I$(updir1)/include
-
-libcygwin:=$(cygwin_build)/libcygwin.a
-libuser32:=$(w32api_lib)/libuser32.a
-libkernel32:=$(w32api_lib)/libkernel32.a
-ALL_DEP_LDLIBS:=$(libcygwin) $(w32api_lib)/libnetapi32.a \
-   $(w32api_lib)/libadvapi32.a $(w32api_lib)/libkernel32.a \
-   $(w32api_lib)/libuser32.a
-
-ALL_LDLIBS:=${patsubst $(w32api_lib)/lib%.a,-l%,\
- ${filter-out $(libuser32),\
-  ${filter-out $(libkernel32),\
-   ${filter-out $(libcygwin), $(ALL_DEP_LDLIBS)
-
-MINGW_LIB:=$(mingw_build)/libmingw32.a
-DUMPER_LIB:=${libbfd} ${libintl} -L$(bupdir1)/libiberty $(LIBICONV) -liberty
-MINGW_LDLIBS:=${filter-out $(libcygwin),$(ALL_LDLIBS) $(MINGW_LIB)}
-MINGW_DEP_LDLIBS:=${ALL_DEP_LDLIBS} ${MINGW_LIB}
-ALL_LDFLAGS:=-B$(newlib_build)/libc -B$(w32api_lib) $(LDFLAGS) $(ALL_LDLIBS)
-DUMPER_LDFLAGS:=$(ALL_LDFLAGS) $(DUMPER_LIB)
-MINGW_CXX:=${patsubst %/cygwin/include,%/mingw/include,${filter-out 
-I$(newlib_source)/%,$(COMPILE_CXX)}} -I$(updir)
-
-PROGS:=cygcheck.exe cygpath.exe getfacl.exe kill.exe mkgroup.exe \
-   mkpasswd.exe mount.exe passwd.exe ps.exe regtool.exe setfacl.exe \
-   setmetamode.exe ssp.exe strace.exe umount.exe ipcrm.exe ipcs.exe
-
-CLEAN_PROGS:=$(PROGS)
-ifndef build_dumper
-PROGS:=warn_dumper $(PROGS)
-else
-PROGS+=dumper$(EXEEXT)
-CLEAN_PROGS+=dumper.exe
-endif
-
 .SUFFIXES:
 .NOEXPORT:
+.PHONY: all install clean realclean warn_dumper warn_cygcheck_zlib
 
-.PHONY: all install clean realclean warn_dumper
-
-all: Makefile $(PROGS)
-
-strace.exe: strace.o path.o $(MINGW_DEP_LDLIBS)
-ifdef VERBOSE
-   $(CXX) $(MINGW_CXXFLAGS) -o $@ ${wordlist 1,2,$^} -B$(mingw_build)/ 
$(MINGW_LDFLAGS)
-else
-   @echo $(CXX) -o $@ ${wordlist 1,2,$^} ${filter-out -B%, 
$(MINGW_CXXFLAGS) $(MINGW_LDFLAGS)};\
-   $(CXX) $(MINGW_CXXFLAGS) -o $@ ${wordlist 1,2,$^} -B$(mingw_build)/ 
$(MINGW_LDFLAGS)
-endif
-
-cygcheck.exe: cygcheck.o bloda.o path.o dump_setup.o $(MINGW_DEP_LDLIBS)
-ifeq $(libz) 
-   @echo '*** Building cygcheck without package content checking due to 
missing mingw libz.a.'
-endif
-ifdef VERBOSE
-   

Re: [patch] reorganize utils/Makefile.in

2008-03-08 Thread Christopher Faylor
On Sat, Mar 08, 2008 at 07:28:52AM -0800, Brian Dessent wrote:
2008-03-08  Brian Dessent  [EMAIL PROTECTED]

   * Makefile.in: Reorganize considerably, using GNU make's
   static pattern rules and target-specific variables.

Looks good.  Please check in.

Thanks.

cgf


[patch] utils/path.cc fixes and testsuite

2008-03-08 Thread Brian Dessent

The winsup/utils/path.cc file implements a primative POSIX-Win32 path
conversion API that is independant of the real one in Cygwin.  This is
used by cygcheck as well as strace (for opening the -o parameter). 
Currently this code has bitrotted a bit, and its handling of relative
paths seems to be a little awkward.  Examples of how it's broken:

strace -o foo.out cmd will place foo.out file in / (or some other
strange location, usually the root of whatever prefix of the mount table
matched CWD) instead of the CWD.

cd /bin  cygcheck ls reports Error: could not find ls, but cd
/bin  cygcheck ./ls does work.

cd /  cygcheck usr/bin/ls reports usr/bin/ls - Cannot open but cd
/  cygcheck bin/ls works.

cp /bin/ls /tmp  cd /  cygcheck tmp/ls reports tmp/ls - Cannot
open.

Anyway, I took a look at fixing this so that the path.cc code is a
little more robust.  But as I'm sure you're aware, messing with path
conversion code is really annoying when you factor in all sorts of weird
corner cases, so I decided to make a testsuite for this code so that it
would be possible to both cure the bitrot as well as know if it has
regressed again.  That attached patch does both.

The harness that I came up with consists of simply a header
(testsuite.h) which contains definitions for both a toy mount table as
well as a series of {CWD,POSIX input,expected Win32 output} tuples that
form the tests.  The driver is testsuite.c, and it works by recompiling
path.cc with -DTESTSUITE to activate the hooks.  It's pretty simplistic,
but it has allowed me to fix a number of corner cases in the code such
that all of the above mentioned oddities are now gone.  In order to make
the testsuite pass I also had to add a little bit of normalization code
so that e.g. the Win32 output always uses backslashes and that it
doesn't leave repeated slashes between parts, e.g. foo\\/bar.  For the
sample mount table I tried to emulate what a real mount table is
typically like, with a couple additional mounts.

The Makefile changes to support this are pretty straightforward: nothing
changes for the default make all case.  If you run make check it
builds the required files and runs the testsuite.  I've attached a
sample output.

Brian2008-03-08  Brian Dessent  [EMAIL PROTECTED]

* Makefile.in: Add a 'check' target that builds and runs
testsuite.exe from path-testsuite.o and testsuite.o.
* path.cc: Include testsuite.h.
(struct mnt): Change to a mnt_t typedef and don't define
mount_table when TESTSUITE is defined.
(find2): Don't include when TESTSUITE is defined to avoid warning.
(get_cygdrive0): Ditto.
(get_cygdrive): Ditto.
(read_mounts): Provide empty implementation when TESTSUITE is
defined.
(vconcat): Use the isslash macro.
(unconvert_slashes): New helper to convert to backslashses.
(rel_vconcat): Handle relative paths more gracefully.
(cygpath): Skip a leading ./ sequence.  Avoid double-slashes.
Normalize final output to backslashes and remove redundant path
sequences.
* testsuite.cc: New file implementing testsuite driver.
* testsuite.h: New header implementing harness mount table and
series of tests.

 Makefile.in  |   19 +++-
 path.cc  |   57 +
 testsuite.cc |   89 
 testsuite.h  |  130 +++
 4 files changed, 283 insertions(+), 12 deletions(-)

Index: Makefile.in
===
RCS file: /cvs/src/src/winsup/utils/Makefile.in,v
retrieving revision 1.69
diff -u -p -r1.69 Makefile.in
--- Makefile.in 8 Mar 2008 17:52:49 -   1.69
+++ Makefile.in 8 Mar 2008 18:11:10 -
@@ -99,10 +99,23 @@ else
 all: warn_cygcheck_zlib
 endif
 
-# the rest of this file contains generic rules
-
 all: Makefile $(CYGWIN_BINS) $(MINGW_BINS)
 
+# test harness support (note: the MINGW_BINS += should come after the
+# all: above so that the testsuite is not run for make but only
+# make check.)
+MINGW_BINS += testsuite.exe
+MINGW_OBJS += path-testsuite.o testsuite.o
+testsuite.exe: path-testsuite.o
+path-testsuite.cc: path.cc ; ln -sf ${filter %.cc,$^} $@
+path-testsuite.o: MINGW_CXXFLAGS += -DTESTSUITE
+# this is necessary because this .c lives in the build dir instead of src
+path-testsuite.o: MINGW_CXX := ${patsubst -I.,-I$(utils_source),$(MINGW_CXX)}
+path-testsuite.cc path.cc testsuite.cc: testsuite.h
+check: testsuite.exe ; $(D)/$(F)
+
+# the rest of this file contains generic rules
+
 # how to compile a MinGW object
 $(MINGW_OBJS): %.o: %.cc
 ifdef VERBOSE
@@ -137,7 +150,7 @@ $(MINGW_BINS): $(MINGW_DEP_LDLIBS)
 $(CYGWIN_BINS): $(ALL_DEP_LDLIBS)
 
 clean:
-   rm -f *.o $(CYGWIN_BINS) $(MINGW_BINS)
+   rm -f *.o $(CYGWIN_BINS) $(MINGW_BINS) path-testsuite.cc testsuite.exe
 
 realclean: clean
rm -f Makefile config.cache
Index: 

Re: [patch] utils/path.cc fixes and testsuite

2008-03-08 Thread Christopher Faylor
On Sat, Mar 08, 2008 at 11:01:32AM -0800, Brian Dessent wrote:

The winsup/utils/path.cc file implements a primative POSIX-Win32 path
conversion API that is independant of the real one in Cygwin.  This is
used by cygcheck as well as strace (for opening the -o parameter). 
Currently this code has bitrotted a bit, and its handling of relative
paths seems to be a little awkward.  Examples of how it's broken:

strace -o foo.out cmd will place foo.out file in / (or some other
strange location, usually the root of whatever prefix of the mount table
matched CWD) instead of the CWD.

cd /bin  cygcheck ls reports Error: could not find ls, but cd
/bin  cygcheck ./ls does work.

cd /  cygcheck usr/bin/ls reports usr/bin/ls - Cannot open but cd
/  cygcheck bin/ls works.

cp /bin/ls /tmp  cd /  cygcheck tmp/ls reports tmp/ls - Cannot
open.

Anyway, I took a look at fixing this so that the path.cc code is a
little more robust.  But as I'm sure you're aware, messing with path
conversion code is really annoying when you factor in all sorts of weird
corner cases, so I decided to make a testsuite for this code so that it
would be possible to both cure the bitrot as well as know if it has
regressed again.  That attached patch does both.

The harness that I came up with consists of simply a header
(testsuite.h) which contains definitions for both a toy mount table as
well as a series of {CWD,POSIX input,expected Win32 output} tuples that
form the tests.  The driver is testsuite.c, and it works by recompiling
path.cc with -DTESTSUITE to activate the hooks.  It's pretty simplistic,
but it has allowed me to fix a number of corner cases in the code such
that all of the above mentioned oddities are now gone.  In order to make
the testsuite pass I also had to add a little bit of normalization code
so that e.g. the Win32 output always uses backslashes and that it
doesn't leave repeated slashes between parts, e.g. foo\\/bar.  For the
sample mount table I tried to emulate what a real mount table is
typically like, with a couple additional mounts.

The Makefile changes to support this are pretty straightforward: nothing
changes for the default make all case.  If you run make check it
builds the required files and runs the testsuite.  I've attached a
sample output.

Brian
2008-03-08  Brian Dessent  [EMAIL PROTECTED]

   * Makefile.in: Add a 'check' target that builds and runs
   testsuite.exe from path-testsuite.o and testsuite.o.
   * path.cc: Include testsuite.h.
   (struct mnt): Change to a mnt_t typedef and don't define
   mount_table when TESTSUITE is defined.
   (find2): Don't include when TESTSUITE is defined to avoid warning.
   (get_cygdrive0): Ditto.
   (get_cygdrive): Ditto.
   (read_mounts): Provide empty implementation when TESTSUITE is
   defined.
   (vconcat): Use the isslash macro.
   (unconvert_slashes): New helper to convert to backslashses.
   (rel_vconcat): Handle relative paths more gracefully.
   (cygpath): Skip a leading ./ sequence.  Avoid double-slashes.
   Normalize final output to backslashes and remove redundant path
   sequences.
   * testsuite.cc: New file implementing testsuite driver.
   * testsuite.h: New header implementing harness mount table and
   series of tests.

 Makefile.in  |   19 +++-
 path.cc  |   57 +
 testsuite.cc |   89 
 testsuite.h  |  130 
 +++
 4 files changed, 283 insertions(+), 12 deletions(-)

Index: Makefile.in
===
RCS file: /cvs/src/src/winsup/utils/Makefile.in,v
retrieving revision 1.69
diff -u -p -r1.69 Makefile.in
--- Makefile.in8 Mar 2008 17:52:49 -   1.69
+++ Makefile.in8 Mar 2008 18:11:10 -
@@ -99,10 +99,23 @@ else
 all: warn_cygcheck_zlib
 endif
 
-# the rest of this file contains generic rules
-
 all: Makefile $(CYGWIN_BINS) $(MINGW_BINS)
 
+# test harness support (note: the MINGW_BINS += should come after the
+# all: above so that the testsuite is not run for make but only
+# make check.)
+MINGW_BINS += testsuite.exe
+MINGW_OBJS += path-testsuite.o testsuite.o
+testsuite.exe: path-testsuite.o
+path-testsuite.cc: path.cc ; ln -sf ${filter %.cc,$^} $@
+path-testsuite.o: MINGW_CXXFLAGS += -DTESTSUITE
+# this is necessary because this .c lives in the build dir instead of src
+path-testsuite.o: MINGW_CXX := ${patsubst -I.,-I$(utils_source),$(MINGW_CXX)}
+path-testsuite.cc path.cc testsuite.cc: testsuite.h
+check: testsuite.exe ; $(D)/$(F)
+
+# the rest of this file contains generic rules
+
 # how to compile a MinGW object
 $(MINGW_OBJS): %.o: %.cc
 ifdef VERBOSE
@@ -137,7 +150,7 @@ $(MINGW_BINS): $(MINGW_DEP_LDLIBS)
 $(CYGWIN_BINS): $(ALL_DEP_LDLIBS)
 
 clean:
-  rm -f *.o $(CYGWIN_BINS) $(MINGW_BINS)
+  rm -f *.o $(CYGWIN_BINS) $(MINGW_BINS) path-testsuite.cc testsuite.exe
 
 

Re: [patch] utils/path.cc fixes and testsuite

2008-03-08 Thread Brian Dessent
Christopher Faylor wrote:

 Would it be possible to uncollapse this if block and make it a little
 clearer?  The else nine lines away makes it a little hard to follow.
 So, wouldn't just inverting the if (match) to if (!match) and
 putting the else condition cause some unnesting?

Here's a v2 patch.  Changes:

- As suggested I used path[max_len] instead of *(path + max_len).  I had
done it that way originally to try to make it clear that I was looking
at what the first chararacter of the path + max_len argument to
concat, but I now agree it's kind of unidiomatic C.

- Rearranged the if/else block, and added a comment for the logic of
each branch.

- Added a test for UNC paths.

- Minor tweak on the Makefile rule that symlinks the source file to
another name to prevent it from running every time.  In general I'm not
all that crazy with this idea of symlinking a file in order to compile
it to a differently-named object, but doing it otherwise would require
repeating the compile rule with all its ugly VERBOSE casing and I just
went to the trouble to eliminate all such repetition in the Makefile.

Brian2008-03-08  Brian Dessent  [EMAIL PROTECTED]

* Makefile.in: Add a 'check' target that builds and runs
testsuite.exe from path-testsuite.o and testsuite.o.
* path.cc: Include testsuite.h.
(struct mnt): Change to a mnt_t typedef and don't define
mount_table when TESTSUITE is defined.
(find2): Don't include when TESTSUITE is defined to avoid warning.
(get_cygdrive0): Ditto.
(get_cygdrive): Ditto.
(read_mounts): Provide empty implementation when TESTSUITE is
defined.
(vconcat): Use the isslash macro.
(unconvert_slashes): New helper to convert to backslashses.
(rel_vconcat): Handle relative paths more gracefully.
(cygpath): Skip a leading ./ sequence.  Avoid double-slashes.
Normalize final output to backslashes and remove redundant path
sequences.
* testsuite.cc: New file implementing testsuite driver.
* testsuite.h: New header implementing harness mount table and
series of tests.

 Makefile.in  |   19 +++-
 path.cc  |   71 +++
 testsuite.cc |   89 
 testsuite.h  |  131 +++
 4 files changed, 298 insertions(+), 12 deletions(-)

Index: Makefile.in
===
RCS file: /cvs/src/src/winsup/utils/Makefile.in,v
retrieving revision 1.69
diff -u -p -r1.69 Makefile.in
--- Makefile.in 8 Mar 2008 17:52:49 -   1.69
+++ Makefile.in 9 Mar 2008 03:01:17 -
@@ -99,10 +99,23 @@ else
 all: warn_cygcheck_zlib
 endif
 
-# the rest of this file contains generic rules
-
 all: Makefile $(CYGWIN_BINS) $(MINGW_BINS)
 
+# test harness support (note: the MINGW_BINS += should come after the
+# all: above so that the testsuite is not run for make but only
+# make check.)
+MINGW_BINS += testsuite.exe
+MINGW_OBJS += path-testsuite.o testsuite.o
+testsuite.exe: path-testsuite.o
+path-testsuite.cc: path.cc ; @test -L $@ || ln -sf ${filter %.cc,$^} $@
+path-testsuite.o: MINGW_CXXFLAGS += -DTESTSUITE
+# this is necessary because this .c lives in the build dir instead of src
+path-testsuite.o: MINGW_CXX := ${patsubst -I.,-I$(utils_source),$(MINGW_CXX)}
+path-testsuite.cc path.cc testsuite.cc: testsuite.h
+check: testsuite.exe ; $(D)/$(F)
+
+# the rest of this file contains generic rules
+
 # how to compile a MinGW object
 $(MINGW_OBJS): %.o: %.cc
 ifdef VERBOSE
@@ -137,7 +150,7 @@ $(MINGW_BINS): $(MINGW_DEP_LDLIBS)
 $(CYGWIN_BINS): $(ALL_DEP_LDLIBS)
 
 clean:
-   rm -f *.o $(CYGWIN_BINS) $(MINGW_BINS)
+   rm -f *.o $(CYGWIN_BINS) $(MINGW_BINS) path-testsuite.cc testsuite.exe
 
 realclean: clean
rm -f Makefile config.cache
Index: path.cc
===
RCS file: /cvs/src/src/winsup/utils/path.cc,v
retrieving revision 1.12
diff -u -p -r1.12 path.cc
--- path.cc 4 Jun 2007 01:57:16 -   1.12
+++ path.cc 9 Mar 2008 03:01:17 -
@@ -1,6 +1,6 @@
 /* path.cc
 
-   Copyright 2001, 2002, 2003, 2005, 2006, 2007 Red Hat, Inc.
+   Copyright 2001, 2002, 2003, 2005, 2006, 2007, 2008 Red Hat, Inc.
 
 This file is part of Cygwin.
 
@@ -22,6 +22,7 @@ details. */
 #include cygwin/include/cygwin/version.h
 #include cygwin/include/sys/mount.h
 #include cygwin/include/mntent.h
+#include testsuite.h
 
 /* Used when treating / and \ as equivalent. */
 #define isslash(ch) \
@@ -227,16 +228,27 @@ readlink (HANDLE fh, char *path, int max
   return true;
 }
 
-static struct mnt
+typedef struct mnt
   {
 const char *native;
 char *posix;
 unsigned flags;
 int issys;
-  } mount_table[255];
+  } mnt_t;
+
+#ifndef TESTSUITE
+static mnt_t mount_table[255];
+#else
+#  define TESTSUITE_MOUNT_TABLE
+#  include testsuite.h
+#  undef 

Re: [patch] utils/path.cc fixes and testsuite

2008-03-08 Thread Christopher Faylor
On Sat, Mar 08, 2008 at 07:10:03PM -0800, Brian Dessent wrote:
Christopher Faylor wrote:

 Would it be possible to uncollapse this if block and make it a little
 clearer?  The else nine lines away makes it a little hard to follow.
 So, wouldn't just inverting the if (match) to if (!match) and
 putting the else condition cause some unnesting?

Here's a v2 patch.  Changes:

- As suggested I used path[max_len] instead of *(path + max_len).  I had
done it that way originally to try to make it clear that I was looking
at what the first chararacter of the path + max_len argument to
concat, but I now agree it's kind of unidiomatic C.

- Rearranged the if/else block, and added a comment for the logic of
each branch.

- Added a test for UNC paths.

- Minor tweak on the Makefile rule that symlinks the source file to
another name to prevent it from running every time.  In general I'm not
all that crazy with this idea of symlinking a file in order to compile
it to a differently-named object, but doing it otherwise would require
repeating the compile rule with all its ugly VERBOSE casing and I just
went to the trouble to eliminate all such repetition in the Makefile.

Brian
2008-03-08  Brian Dessent  [EMAIL PROTECTED]

   * Makefile.in: Add a 'check' target that builds and runs
   testsuite.exe from path-testsuite.o and testsuite.o.
   * path.cc: Include testsuite.h.
   (struct mnt): Change to a mnt_t typedef and don't define
   mount_table when TESTSUITE is defined.
   (find2): Don't include when TESTSUITE is defined to avoid warning.
   (get_cygdrive0): Ditto.
   (get_cygdrive): Ditto.
   (read_mounts): Provide empty implementation when TESTSUITE is
   defined.
   (vconcat): Use the isslash macro.
   (unconvert_slashes): New helper to convert to backslashses.
   (rel_vconcat): Handle relative paths more gracefully.
   (cygpath): Skip a leading ./ sequence.  Avoid double-slashes.
   Normalize final output to backslashes and remove redundant path
   sequences.
   * testsuite.cc: New file implementing testsuite driver.
   * testsuite.h: New header implementing harness mount table and
   series of tests.

 Makefile.in  |   19 +++-
 path.cc  |   71 +++
 testsuite.cc |   89 
 testsuite.h  |  131 
 +++
 4 files changed, 298 insertions(+), 12 deletions(-)

Index: Makefile.in
===
RCS file: /cvs/src/src/winsup/utils/Makefile.in,v
retrieving revision 1.69
diff -u -p -r1.69 Makefile.in
--- Makefile.in8 Mar 2008 17:52:49 -   1.69
+++ Makefile.in9 Mar 2008 03:01:17 -
@@ -99,10 +99,23 @@ else
 all: warn_cygcheck_zlib
 endif
 
-# the rest of this file contains generic rules
-
 all: Makefile $(CYGWIN_BINS) $(MINGW_BINS)
 
+# test harness support (note: the MINGW_BINS += should come after the
+# all: above so that the testsuite is not run for make but only
+# make check.)
+MINGW_BINS += testsuite.exe
+MINGW_OBJS += path-testsuite.o testsuite.o
+testsuite.exe: path-testsuite.o
+path-testsuite.cc: path.cc ; @test -L $@ || ln -sf ${filter %.cc,$^} $@
+path-testsuite.o: MINGW_CXXFLAGS += -DTESTSUITE
+# this is necessary because this .c lives in the build dir instead of src
+path-testsuite.o: MINGW_CXX := ${patsubst -I.,-I$(utils_source),$(MINGW_CXX)}
+path-testsuite.cc path.cc testsuite.cc: testsuite.h
+check: testsuite.exe ; $(D)/$(F)
+
+# the rest of this file contains generic rules
+
 # how to compile a MinGW object
 $(MINGW_OBJS): %.o: %.cc
 ifdef VERBOSE
@@ -137,7 +150,7 @@ $(MINGW_BINS): $(MINGW_DEP_LDLIBS)
 $(CYGWIN_BINS): $(ALL_DEP_LDLIBS)
 
 clean:
-  rm -f *.o $(CYGWIN_BINS) $(MINGW_BINS)
+  rm -f *.o $(CYGWIN_BINS) $(MINGW_BINS) path-testsuite.cc testsuite.exe
 
 realclean: clean
   rm -f Makefile config.cache
Index: path.cc
===
RCS file: /cvs/src/src/winsup/utils/path.cc,v
retrieving revision 1.12
diff -u -p -r1.12 path.cc
--- path.cc4 Jun 2007 01:57:16 -   1.12
+++ path.cc9 Mar 2008 03:01:17 -
@@ -1,6 +1,6 @@
 /* path.cc
 
-   Copyright 2001, 2002, 2003, 2005, 2006, 2007 Red Hat, Inc.
+   Copyright 2001, 2002, 2003, 2005, 2006, 2007, 2008 Red Hat, Inc.
 
 This file is part of Cygwin.
 
@@ -22,6 +22,7 @@ details. */
 #include cygwin/include/cygwin/version.h
 #include cygwin/include/sys/mount.h
 #include cygwin/include/mntent.h
+#include testsuite.h
 
 /* Used when treating / and \ as equivalent. */
 #define isslash(ch) \
@@ -227,16 +228,27 @@ readlink (HANDLE fh, char *path, int max
   return true;
 }
 
-static struct mnt
+typedef struct mnt
   {
 const char *native;
 char *posix;
 unsigned flags;
 int issys;
-  } mount_table[255];
+  } mnt_t;
+
+#ifndef TESTSUITE
+static mnt_t mount_table[255];
+#else
+#  define 

Re: [patch] cygcheck.cc update for cygpath()

2008-03-08 Thread Brian Dessent
Christopher Faylor wrote:

 It looks like yo can still unindent this by  changing the == to !=, putting
 the temppath under that and keeping all of the if's at the same level:

Oh, I see now what you mean.

 If the if block is that small, then I think I'd prefer just one comment
 at the beginning which describes what it is doing.  Otherwise, I got
 lost in what was happening while trying to see where the comments line
 up.  I don't feel really strongly about that, though, so feel free to
 ignore me.  I would prefer not having the nested if's though.
 
 Otherwise, this looks good.  If you make the above suggestions, feel
 free to check this in.

I chopped each comment down to a single line //-style which I think
helps the clutter, and removed the nesting.

Also, there are a few tweaks to cygcheck.cc necessary as a result, see
attached.  The main idea is that when normalizing a link's target in
find_app_on_path, we should temporarily set the CWD to the same dir as
the link, otherwise relative links will get normalized relative to
whatever dir cygcheck was run from.  

Brian2008-03-08  Brian Dessent  [EMAIL PROTECTED]

* cygcheck.cc (save_cwd_helper): New helper function for saving
the current directory.
(find_app_on_path): Use sizeof instead of hardcoded array sizes
throughout.  Change into the dir of the link when normalizing
its target.  Don't worry about converting slashes as cygpath ()
now handles that.

 cygcheck.cc |   45 -
 1 file changed, 36 insertions(+), 9 deletions(-)

Index: cygcheck.cc
===
RCS file: /cvs/src/src/winsup/utils/cygcheck.cc,v
retrieving revision 1.97
diff -u -p -r1.97 cygcheck.cc
--- cygcheck.cc 13 Jan 2008 13:41:45 -  1.97
+++ cygcheck.cc 9 Mar 2008 03:52:07 -
@@ -807,6 +807,31 @@ ls (char *f)
 display_error (ls: CloseHandle());
 }
 
+/* If s is non-NULL, save the CWD in a static buffer and set the CWD
+   to the dirname part of s.  If s is NULL, restore the CWD last
+   saved.  */
+static
+void save_cwd_helper (const char *s)
+{
+  static char cwdbuf[MAX_PATH + 1];
+  char dirnamebuf[MAX_PATH + 1];
+
+  if (s)
+{
+  GetCurrentDirectory (sizeof (cwdbuf), cwdbuf);
+
+  /* Remove the filename part from s.  */
+  strncpy (dirnamebuf, s, MAX_PATH);
+  dirnamebuf[MAX_PATH] = '\0';   // just in case strlen(s)  MAX_PATH
+  char *lastsep = strrchr (dirnamebuf, '\\');
+  if (lastsep)
+lastsep[1] = '\0';
+  SetCurrentDirectory (dirnamebuf);
+}
+  else
+SetCurrentDirectory (cwdbuf);
+}
+
 // Find a real application on the path (possibly following symlinks)
 static const char *
 find_app_on_path (const char *app, bool showall = false)
@@ -822,25 +847,27 @@ find_app_on_path (const char *app, bool 
   if (is_symlink (fh))
 {
   static char tmp[4000] = ;
-  char *ptr;
-  if (!readlink (fh, tmp, 3999))
+  if (!readlink (fh, tmp, sizeof (tmp)-1))
display_error(readlink failed);
-  ptr = cygpath (tmp, NULL);
-  for (char *p = ptr; (p = strchr (p, '/')); p++)
-   *p = '\\';
+  
+  /* When resolving the linkname, set the CWD to the context of
+ the link, so that relative links are correctly resolved.  */
+  save_cwd_helper (papp);
+  char *ptr = cygpath (tmp, NULL);
+  save_cwd_helper (NULL);
   printf ( - %s\n, ptr);
   if (!strchr (ptr, '\\'))
{
  char *lastsep;
- strncpy (tmp, cygpath (papp, NULL), 3999);
- for (char *p = tmp; (p = strchr (p, '/')); p++)
-   *p = '\\';
+ strncpy (tmp, cygpath (papp, NULL), sizeof (tmp)-1);
  lastsep = strrchr (tmp, '\\');
- strncpy (lastsep+1, ptr, 3999-(lastsep-tmp));
+ strncpy (lastsep+1, ptr, (sizeof (tmp)-1) - (lastsep-tmp));
  ptr = tmp;
}
   if (!CloseHandle (fh))
display_error (find_app_on_path: CloseHandle());
+  /* FIXME: We leak the ptr returned by cygpath() here which is a
+ malloc()d string.  */
   return find_app_on_path (ptr, showall);
 }