On 02/04/2011 10:56 AM, Jan Nieuwenhuizen wrote: > Hi, > > See attached, please apply.
> Subject: [PATCH 2/4] canonicalize-lgpl: Add basic sanity checks for mingw > demonstrating general breakage. I'm not a fan of summary lines longer than 80 columns. I would have done: canonicalize-lgpl: add mingw tests > > 2011-02-04 Jan Nieuwenhuizen <[email protected]> > > * tests/test-canonicalize-lgpl.c (main)[(_WIN32 || __WIN32__) && ! > __CYGWIN__]: In the code, you need the full condition; but in changelog entries, I'm fine with abbreviating this particular idiom: * tests/test-canonicalize-lgpl.c (main) [WIN32]: Add ... > Add basic sanity checks for mingw along with debug printing > demonstrating > general breakage. For git bisectability, I don't like to commit known broken tests without also committing their fixes; either as one commit, or as fixed test second. But that's not as much an issue here in gnulib (where any end user is unlikely to ever pick a broken commit), so I'm not too fussed about your patch order. > > +#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__ > + /* Check basic drive letter sanity. */ > + { > + char *cwd; > + char *test; > + char *result; > + > + /* Check if BASE has a canonical name. */ > + result = canonicalize_file_name (BASE); > + fprintf (stderr, "BASE-canon: %s\n", result); Oops - you've left debug statements in. A successful test should be silent. > + ASSERT (result != NULL); > + > + /* Check if BASE's canonical name is somewhat canonical. */ > + ASSERT ((strchr (result, '/') == NULL) > + != (strchr (result, '\\') == NULL)); A canonical name for win32 should use ONLY \, never /. Insist on that, otherwise, you won't be able to compare two path names that differ only in their choice of (otherwise-synonymous) directory separators. The macro DIRECTORY_SEPARATOR (guaranteed by at least lib/dirname.h, with '/' for Unix-y systems, including Cygwin, and '\\' for mingw) may be worth using more liberally throughout this file, especially outside the mingw-specific portion of the test. > + > + /* Check if CWD has a canonical name. */ > + cwd = getcwd (NULL, 0); > + fprintf (stderr, "CWD: %s\n", cwd); > + result = canonicalize_file_name (cwd); > + fprintf (stderr, "CWD-canon: %s\n", result); Two more debug statements that should be removed. > + ASSERT (result != NULL); Where are you freeing result? It's wise to avoid memory leaks, even in simple tests. -- Eric Blake [email protected] +1-801-349-2682 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
