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

[Ãvar ArnfjÃrà Bjarmason <avarab@xxxxxxxxx>]

On Sun, Oct 3, 2010 at 04:32, Joshua Jensen <jjensen@xxxxxxxxxxxxxxxxx> wrote:

> diff --git a/dir.c b/dir.c
> index d1e5e5e..ffa410d 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -18,6 +18,68 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, in
> Â Â Â Âint check_only, const struct path_simplify *simplify);
> Âstatic int get_dtype(struct dirent *de, const char *path, int len);
>
> +/* helper string functions with support for the ignore_case flag */
> +int strcmp_icase(const char *a, const char *b)
> +{
> + Â Â Â return ignore_case ? strcasecmp(a, b) : strcmp(a, b);
> +}
> +
> +int strncmp_icase(const char *a, const char *b, size_t count)
> +{
> + Â Â Â return ignore_case ? strncasecmp(a, b, count) : strncmp(a, b, count);
> +}
> +
> +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.

That would allow the GNU libc, FreeBSD libc and others that implement
the GNU extension to do case folding for us, and we wouldn't have to
maintain our own fnmatch_casefold.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html