Re: [PATCH v2 1/6] Add string comparison functions that respect the ignore_case variable.

  ----- Original Message -----
From: Ævar Arnfjörð Bjarmason
Date: 10/3/2010 2:30 AM

On Sun, Oct 3, 2010 at 04:32, Joshua Jensen[off-list ref]  wrote

quoted

+int fnmatch_casefold(const char *pattern, const char *string, int flags)
+{
+       char lowerPatternBuf[MAX_PATH];
+       char lowerStringBuf[MAX_PATH];
+       char* lowerPattern;
+       char* lowerString;
+       size_t patternLen;
+       size_t stringLen;
+       char* out;
+       int ret;
+
+       /*
+        * Use the provided stack buffer, if possible.  If the string is too
+        * large, allocate buffer space.
+        */
+       patternLen = strlen(pattern);
+       if (patternLen + 1>  sizeof(lowerPatternBuf))
+               lowerPattern = xmalloc(patternLen + 1);
+       else
+               lowerPattern = lowerPatternBuf;
+
+       stringLen = strlen(string);
+       if (stringLen + 1>  sizeof(lowerStringBuf))
+               lowerString = xmalloc(stringLen + 1);
+       else
+               lowerString = lowerStringBuf;
+
+       /* Make the pattern and string lowercase to pass to fnmatch. */
+       for (out = lowerPattern; *pattern; ++out, ++pattern)
+               *out = tolower(*pattern);
+       *out = 0;
+
+       for (out = lowerString; *string; ++out, ++string)
+               *out = tolower(*string);
+       *out = 0;
+
+       ret = fnmatch(lowerPattern, lowerString, flags);
+
+       /* Free the pattern or string if it was allocated. */
+       if (lowerPattern != lowerPatternBuf)
+               free(lowerPattern);
+       if (lowerString != lowerStringBuf)
+               free(lowerString);
+       return ret;
+}
+
+int fnmatch_icase(const char *pattern, const char *string, int flags)
+{
+       return ignore_case ? fnmatch_casefold(pattern, string, flags) : fnmatch(pattern, string, flags);
+}

I liked v1 of this patch better, although it obviously had portability
issues. But I think it would be better to handle this with:

     #ifndef FNM_CASEFOLD
     int fnmatch_casefold(const char *pattern, const char *string, int flags)
     {
         ...
     }
     #endf

     int fnmatch_icase(const char *pattern, const char *string, int flags)
     {
     #ifndef FNM_CASEFOLD
            return ignore_case ? fnmatch_casefold(pattern, string,
flags) : fnmatch(pattern, string, flags);
     #else
             return fnmatch(pattern, string, flags | (ignore_case ?
FNM_CASEFOLD : 0));
     #endif
     }

Or simply use fnmatch(..., FNM_CASEFOLD) everywhere and include
compat/fnmatch/* on platforms like Solaris that don't have the GNU
extension.

The real problem with compat/fnmatch is determining which random 
platforms need that support and updating the makefile accordingly.  
Further, the compat/fnmatch/* code would need to be rejigged somewhat, 
so there is no possible conflict (now or in the future) with the 
provided symbols.  We discussed this as a potential problem developers 
would need to be aware of if the system fnmatch.h (or whatever it is 
called) gets #included.

Anyway, what you describe above creates two code paths.  I would imagine 
that would be harder to debug; that is, on some platforms, it uses 
fnmatch_casefold and on others, it hands it off to fnmatch(..., 
FNM_CASEFOLD).

In any case, I'd like to find a solution to get this series working for 
everyone.  I've been out of commission for a month (deploying Git to 80+ 
programmers at an organization, by the way), but I'm back now and can 
work this until it is complete.

Josh