Re: [PATCH 1/3] grep: do lowercase conversion in print_line_middle only for single-byte case

[Jim Meyering]

Paolo Bonzini wrote:
> From: Norihirio Tanaka <address@hidden>
>
> * src/main.c (print_line_middle): Restrict match_icase code
> to MB_CUR_MAX == 1.  Adjust comments.
> ---
>  src/main.c |   11 ++++++++---
>  1 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/src/main.c b/src/main.c
> index e6be36b..b52ad47 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -727,13 +727,18 @@ print_line_middle (const char *beg, const char *lim,
>    char *buf;         /* XXX */
>    const char *ibeg;  /* XXX */
>
> -  if (match_icase)   /* XXX - None of the -i stuff should be here.  */
> +  /* XXX - dfasearch.c and kwsearch.c are already doing the same for
> +     MB_CUR_MAX > 1.  Maybe we should do that unconditionally there
> +     to avoid dealing with match_icase here.  */
> +  if (match_icase
> +#ifdef MBS_SUPPORT
> +      && MB_CUR_MAX == 1
> +#endif
> +     )
>      {
>        int i = lim - beg;
>
>        ibeg = buf = xmalloc(i);
> -      /* This can't possibly be correct with UTF-8,
> -      but it's equivalent to what was there so far.  */
>        while (--i >= 0)
>       buf[i] = tolower((unsigned char) beg[i]);
>      }

Paolo, Norihirio,

I've been going through this in the debugger and have been unable to find
a case in which converting to lower case has any effect on the result,
given single-byte inputs.

In fact, as comments already suggest, I now believe that removing
that entire if-else block is correct.  FYI-patch below.  However, as
far as I can see, that is merely a clean-up and efficiency-improvement
change, and not a bug-fix, so I will defer it.

Paolo, you're welcome to push the above change, even if
it doesn't officially fix a bug, just because it avoids
doing something so obviously nonsensical.


diff --git a/src/main.c b/src/main.c
index e6be36b..309e3a1 100644
--- a/src/main.c
+++ b/src/main.c
@@ -724,28 +724,10 @@ print_line_middle (const char *beg, const char *lim,
   size_t match_offset;
   const char *cur = beg;
   const char *mid = NULL;
-  char *buf;           /* XXX */
-  const char *ibeg;    /* XXX */
-
-  if (match_icase)     /* XXX - None of the -i stuff should be here.  */
-    {
-      int i = lim - beg;
-
-      ibeg = buf = xmalloc(i);
-      /* This can't possibly be correct with UTF-8,
-        but it's equivalent to what was there so far.  */
-      while (--i >= 0)
-       buf[i] = tolower((unsigned char) beg[i]);
-    }
-  else
-    {
-      buf = NULL;
-      ibeg = beg;
-    }

   while (   lim > cur
-        && ((match_offset = execute(ibeg, lim - beg, &match_size,
-                                    ibeg + (cur - beg))) != (size_t) -1))
+        && ((match_offset = execute(beg, lim - beg, &match_size,
+                                    beg + (cur - beg))) != (size_t) -1))
     {
       char const *b = beg + match_offset;

@@ -789,8 +771,6 @@ print_line_middle (const char *beg, const char *lim,
       cur = b + match_size;
     }

-  free (buf);  /* XXX */
-
   if (only_matching)
     cur = lim;
   else if (mid)