Re: grep-2.6 is imminent: pending patches, bug reports?

[Jim Meyering]

Aharon Robbins wrote:
> I think the Debian guys sent me the bug report and I sent them a fix.
>
> Glad you were able to put it into grep.

Thanks.
I've noted that the patch was from you, then.

I've begun to synchronize these two dfa.c files.

Here are some of the easy changes that seem appropriate for gawk.
They're mostly because I insist on warning-free builds in grep
(enabled when configuring with --enable-gcc-warnings, which turns
on likes of -Wshadow).

I'm submitting them in git format-patch form, which puts the
first line of the ChangeLog in the Subject: and the remainder
in the body, so this first one would look like this in ChangeLog:

        build: avoid shadowing warnings
        * dfa.c (match_mb_charset): Rename parameter: s/index/idx/.
        (check_matching_with_multibyte_ops, match_anychar): Likewise.

If you'd prefer some other form, let me know.

The meatier changes are all in the gawk->grep direction,
and I'll work through those as I find time and test cases
to exercise whatever flaw they seem to fix.

Since this is "git format-patch" format, if gawk were using
git ;-), you could apply the entire series with "git am FILE".

>From 1a4806a05c61137d41f46e2d0db06a0eb2e860eb Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Fri, 5 Mar 2010 09:48:00 +0100
Subject: [PATCH 1/5] build: avoid shadowing warnings

* dfa.c (match_mb_charset): Rename parameter: s/index/idx/.
(check_matching_with_multibyte_ops, match_anychar): Likewise.
---
 dfa.c |   30 +++++++++++++++---------------
 1 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/dfa.c b/dfa.c
index b02955e..8e1fd28 100644
--- a/dfa.c
+++ b/dfa.c
@@ -2527,18 +2527,18 @@ transit_state_singlebyte (struct dfa *d, int s, 
unsigned char const *p,
 /* Check whether period can match or not in the current context.  If it can,
    return the amount of the bytes with which period can match, otherwise
    return 0.
-   `pos' is the position of the period.  `index' is the index from the
+   `pos' is the position of the period.  `idx' is the index from the
    buf_begin, and it is the current position in the buffer.  */
 static int
-match_anychar (struct dfa *d, int s, position pos, int index)
+match_anychar (struct dfa *d, int s, position pos, int idx)
 {
   int newline = 0;
   int letter = 0;
   wchar_t wc;
   int mbclen;

-  wc = inputwcs[index];
-  mbclen = (mblen_buf[index] == 0)? 1 : mblen_buf[index];
+  wc = inputwcs[idx];
+  mbclen = (mblen_buf[idx] == 0)? 1 : mblen_buf[idx];

   /* Check context.  */
   if (wc == (wchar_t)eolbyte)
@@ -2567,10 +2567,10 @@ match_anychar (struct dfa *d, int s, position pos, int 
index)
 /* Check whether bracket expression can match or not in the current context.
    If it can, return the amount of the bytes with which expression can match,
    otherwise return 0.
-   `pos' is the position of the bracket expression.  `index' is the index
+   `pos' is the position of the bracket expression.  `idx' is the index
    from the buf_begin, and it is the current position in the buffer.  */
 int
-match_mb_charset (struct dfa *d, int s, position pos, int index)
+match_mb_charset (struct dfa *d, int s, position pos, int idx)
 {
   int i;
   int match;           /* Flag which represent that matching succeed.  */
@@ -2587,7 +2587,7 @@ match_mb_charset (struct dfa *d, int s, position pos, int 
index)
   int letter = 0;
   wchar_t wc;          /* Current refering character.  */

-  wc = inputwcs[index];
+  wc = inputwcs[idx];

   /* Check context.  */
   if (wc == (wchar_t)eolbyte)
@@ -2611,7 +2611,7 @@ match_mb_charset (struct dfa *d, int s, position pos, int 
index)
   /* Assign the current refering operator to work_mbc.  */
   work_mbc = &(d->mbcsets[(d->multibyte_prop[pos.index]) >> 2]);
   match = !work_mbc->invert;
-  match_len = (mblen_buf[index] == 0)? 1 : mblen_buf[index];
+  match_len = (mblen_buf[idx] == 0)? 1 : mblen_buf[idx];

   /* match with a character class?  */
   for (i = 0; i<work_mbc->nch_classes; i++)
@@ -2620,14 +2620,14 @@ match_mb_charset (struct dfa *d, int s, position pos, 
int index)
        goto charset_matched;
     }

-  strncpy(buffer, (const char*) buf_begin + index, match_len);
+  strncpy(buffer, (const char*) buf_begin + idx, match_len);
   buffer[match_len] = '\0';

   /* match with an equivalent class?  */
   for (i = 0; i<work_mbc->nequivs; i++)
     {
       op_len = strlen(work_mbc->equivs[i]);
-      strncpy(buffer, (const char *) buf_begin + index, op_len);
+      strncpy(buffer, (const char *) buf_begin + idx, op_len);
       buffer[op_len] = '\0';
       if (strcoll(work_mbc->equivs[i], buffer) == 0)
        {
@@ -2640,7 +2640,7 @@ match_mb_charset (struct dfa *d, int s, position pos, int 
index)
   for (i = 0; i<work_mbc->ncoll_elems; i++)
     {
       op_len = strlen(work_mbc->coll_elems[i]);
-      strncpy(buffer, (const char *) buf_begin + index, op_len);
+      strncpy(buffer, (const char *) buf_begin + idx, op_len);
       buffer[op_len] = '\0';

       if (strcoll(work_mbc->coll_elems[i], buffer) == 0)
@@ -2681,11 +2681,11 @@ match_mb_charset (struct dfa *d, int s, position pos, 
int index)
    array which corresponds to `d->states[s].mbps.elem' and each element of
    the array contains the amount of the bytes with which the element can
    match.
-   `index' is the index from the buf_begin, and it is the current position
+   `idx' is the index from the buf_begin, and it is the current position
    in the buffer.
    Caller MUST free the array which this function return.  */
 static int*
-check_matching_with_multibyte_ops (struct dfa *d, int s, int index)
+check_matching_with_multibyte_ops (struct dfa *d, int s, int idx)
 {
   int i;
   int* rarray;
@@ -2697,10 +2697,10 @@ check_matching_with_multibyte_ops (struct dfa *d, int 
s, int index)
       switch(d->tokens[pos.index])
        {
        case ANYCHAR:
-         rarray[i] = match_anychar(d, s, pos, index);
+         rarray[i] = match_anychar(d, s, pos, idx);
          break;
        case MBCSET:
-         rarray[i] = match_mb_charset(d, s, pos, index);
+         rarray[i] = match_mb_charset(d, s, pos, idx);
          break;
        default:
          break; /* can not happen.  */
-- 
1.7.0.1.300.gd855a


>From b79103d02d90cdc90719ad8083f06b2459aed059 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Fri, 5 Mar 2010 09:49:01 +0100
Subject: [PATCH 2/5] build: avoid shadowing warning for unused "rs"

* dfa.c (transit_state): Remove dead stores;
Ignore transit_state_consume_1char return value.
---
 dfa.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/dfa.c b/dfa.c
index 8e1fd28..5a56d8a 100644
--- a/dfa.c
+++ b/dfa.c
@@ -2776,7 +2776,6 @@ transit_state (struct dfa *d, int s, unsigned char const 
**pp)
   int nelem = d->states[s].mbps.nelem; /* Just a alias.  */
   position_set follows;
   unsigned char const *p1 = *pp;
-  status_transit_state rs;
   wchar_t wc;

   if (nelem > 0)
@@ -2819,7 +2818,7 @@ transit_state (struct dfa *d, int s, unsigned char const 
**pp)
      not be a character but a (multi character) collating element.
      We enumerate all of the positions which `s' can reach by consuming
      `maxlen' bytes.  */
-  rs = transit_state_consume_1char(d, s, pp, match_lens, &mbclen, &follows);
+  transit_state_consume_1char(d, s, pp, match_lens, &mbclen, &follows);

   wc = inputwcs[*pp - mbclen - buf_begin];
   s1 = state_index(d, &follows, wc == L'\n', iswalnum(wc));
@@ -2828,7 +2827,7 @@ transit_state (struct dfa *d, int s, unsigned char const 
**pp)
   while (*pp - p1 < maxlen)
     {
       follows.nelem = 0;
-      rs = transit_state_consume_1char(d, s1, pp, NULL, &mbclen, &follows);
+      transit_state_consume_1char(d, s1, pp, NULL, &mbclen, &follows);

       for (i = 0; i < nelem ; i++)
        {
-- 
1.7.0.1.300.gd855a


>From 4fec10ab3b53d97b44e29b2965b883088e8d433d Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Fri, 5 Mar 2010 09:50:18 +0100
Subject: [PATCH 3/5] syntax: remove trailing blanks

* dfa.c: Remove trailing blanks, to ease synchronization with grep.
---
 dfa.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/dfa.c b/dfa.c
index 5a56d8a..a0b0a72 100644
--- a/dfa.c
+++ b/dfa.c
@@ -574,7 +574,7 @@ parse_bracket_exp_mb ()
                                       work_mbc->nchars + 2);
                  work_mbc->chars[work_mbc->nchars++] = L'[';
                  work_mbc->chars[work_mbc->nchars++] = delim;
-                 break; 
+                 break;
                }

              if (--lexleft, *lexptr++ != ']')
@@ -2715,7 +2715,7 @@ check_matching_with_multibyte_ops (struct dfa *d, int s, 
int idx)
    of check_matching_with_multibyte_ops() for optimization.
    `mbclen' and `pps' are the output.  `mbclen' is the length of the
    character consumed, and `pps' is the set this function enumerate.  */
-static status_transit_state 
+static status_transit_state
 transit_state_consume_1char (struct dfa *d, int s, unsigned char const **pp,
                             int *match_lens, int *mbclen, position_set *pps)
 {
-- 
1.7.0.1.300.gd855a


>From 8e39521e50cc1836392ad4eace29c360f82d5429 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Fri, 5 Mar 2010 09:51:37 +0100
Subject: [PATCH 4/5] clean-up: limit visibility of an internal function

* dfa.c (match_mb_charset): Declare static.
---
 dfa.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/dfa.c b/dfa.c
index a0b0a72..38993ee 100644
--- a/dfa.c
+++ b/dfa.c
@@ -2569,7 +2569,7 @@ match_anychar (struct dfa *d, int s, position pos, int 
idx)
    otherwise return 0.
    `pos' is the position of the bracket expression.  `idx' is the index
    from the buf_begin, and it is the current position in the buffer.  */
-int
+static int
 match_mb_charset (struct dfa *d, int s, position pos, int idx)
 {
   int i;
-- 
1.7.0.1.300.gd855a


>From 54035fc5104b34b066fa0088c97adcedf517fda0 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Fri, 5 Mar 2010 09:54:11 +0100
Subject: [PATCH 5/5] build: rename local to avoid shadowing global, dfa

* dfa.c (dfamust): Rename parameter: s/dfa/d/.
---
 dfa.c |   24 ++++++++++++------------
 1 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/dfa.c b/dfa.c
index 38993ee..48f5a74 100644
--- a/dfa.c
+++ b/dfa.c
@@ -3480,7 +3480,7 @@ resetmust (must *mp)
 }

 static void
-dfamust (struct dfa *dfa)
+dfamust (struct dfa *d)
 {
   must *musts;
   must *mp;
@@ -3495,13 +3495,13 @@ dfamust (struct dfa *dfa)

   result = empty_string;
   exact = 0;
-  musts = (must *) malloc((dfa->tindex + 1) * sizeof *musts);
+  musts = (must *) malloc((d->tindex + 1) * sizeof *musts);
   if (musts == NULL)
     return;
   mp = musts;
-  for (i = 0; i <= dfa->tindex; ++i)
+  for (i = 0; i <= d->tindex; ++i)
     mp[i] = must0;
-  for (i = 0; i <= dfa->tindex; ++i)
+  for (i = 0; i <= d->tindex; ++i)
     {
       mp[i].in = (char **) malloc(sizeof *mp[i].in);
       mp[i].left = malloc(2);
@@ -3515,16 +3515,16 @@ dfamust (struct dfa *dfa)
     }
 #ifdef DEBUG
   fprintf(stderr, "dfamust:\n");
-  for (i = 0; i < dfa->tindex; ++i)
+  for (i = 0; i < d->tindex; ++i)
     {
       fprintf(stderr, " %d:", i);
-      prtok(dfa->tokens[i]);
+      prtok(d->tokens[i]);
     }
   putc('\n', stderr);
 #endif
-  for (ri = 0; ri < dfa->tindex; ++ri)
+  for (ri = 0; ri < d->tindex; ++ri)
     {
-      switch (t = dfa->tokens[ri])
+      switch (t = d->tokens[ri])
        {
        case LPAREN:
        case RPAREN:
@@ -3693,7 +3693,7 @@ dfamust (struct dfa *dfa)
        }
 #ifdef DEBUG
       fprintf(stderr, " node: %d:", ri);
-      prtok(dfa->tokens[ri]);
+      prtok(d->tokens[ri]);
       fprintf(stderr, "\n  in:");
       for (i = 0; mp->in[i]; ++i)
        fprintf(stderr, " \"%s\"", mp->in[i]);
@@ -3710,11 +3710,11 @@ dfamust (struct dfa *dfa)
       dm->exact = exact;
       MALLOC(dm->must, char, strlen(result) + 1);
       strcpy(dm->must, result);
-      dm->next = dfa->musts;
-      dfa->musts = dm;
+      dm->next = d->musts;
+      d->musts = dm;
     }
   mp = musts;
-  for (i = 0; i <= dfa->tindex; ++i)
+  for (i = 0; i <= d->tindex; ++i)
     {
       freelist(mp[i].in);
       ifree((char *) mp[i].in);
-- 
1.7.0.1.300.gd855a