From: ville.mattila@...
Date: 2006-05-07T15:43:25+09:00
Subject: Re: Coverity CID 14 15 Patch


>
>Comments suggestions welcome.
>
>Kevin Tew
>Index: regparse.c
>===================================================================
>RCS file: /src/ruby/regparse.c,v
>retrieving revision 1.25
>diff -u -r1.25 regparse.c
>--- regparse.c   23 Mar 2006 12:08:08 -0000    1.25
>+++ regparse.c   6 May 2006 16:58:46 -0000
>@@ -2152,38 +2152,41 @@
>pnum = popular_qualifier_num(p);
>cnum = popular_qualifier_num(c);
>
>-  switch(ReduceTypeTable[cnum][pnum]) {
>-  case RQ_DEL:
>-    *p = *c;
>-    break;
>-  case RQ_A:
>-    p->target = c->target;
>-    p->lower  = 0;  p->upper = REPEAT_INFINITE;  p->greedy = 1;
>-    break;
>-  case RQ_AQ:
>-    p->target = c->target;
>-    p->lower  = 0;  p->upper = REPEAT_INFINITE;  p->greedy = 0;
>-    break;
>-  case RQ_QQ:
>-    p->target = c->target;
>-    p->lower  = 0;  p->upper = 1;  p->greedy = 0;
>-    break;
>-  case RQ_P_QQ:
>-    p->target = cnode;
>-    p->lower  = 0;  p->upper = 1;  p->greedy = 0;
>-    c->lower  = 1;  c->upper = REPEAT_INFINITE;  c->greedy = 1;
>-    return ;
>-    break;
>-  case RQ_PQ_Q:
>-    p->target = cnode;
>-    p->lower  = 0;  p->upper = 1;  p->greedy = 1;
>-    c->lower  = 1;  c->upper = REPEAT_INFINITE;  c->greedy = 0;
>-    return ;
>-    break;
>-  case RQ_ASIS:
>-    p->target = cnode;
>-    return ;
>-    break;
>+  if (pnum >= 0 && cnum >= 0 )

 how about

if (pnum < 0  || cnum < 0)
      return;


This way we don't have change identation. I think we could raise
some exception. It is always good for users to known that something does
not work.

>+  {
>+    switch(ReduceTypeTable[cnum][pnum]) {
>+      case RQ_DEL:
>+        *p = *c;
>+        break;
>+      case RQ_A:
>+        p->target = c->target;
>+        p->lower  = 0;  p->upper = REPEAT_INFINITE;  p->greedy = 1;
>+        break;
>+      case RQ_AQ:
>+        p->target = c->target;
>+        p->lower  = 0;  p->upper = REPEAT_INFINITE;  p->greedy = 0;
>+        break;
>+      case RQ_QQ:
>+        p->target = c->target;
>+        p->lower  = 0;  p->upper = 1;  p->greedy = 0;
>+        break;
>+      case RQ_P_QQ:
>+        p->target = cnode;
>+        p->lower  = 0;  p->upper = 1;  p->greedy = 0;
>+        c->lower  = 1;  c->upper = REPEAT_INFINITE;  c->greedy = 1;
>+        return ;
>+        break;
>+      case RQ_PQ_Q:
>+        p->target = cnode;
>+        p->lower  = 0;  p->upper = 1;  p->greedy = 1;
>+        c->lower  = 1;  c->upper = REPEAT_INFINITE;  c->greedy = 0;
>+        return ;
>+        break;
>+      case RQ_ASIS:
>+        p->target = cnode;
>+        return ;
>+        break;
>+    }
>}
>
>c->target = NULL_NODE;
>@@ -4532,32 +4535,34 @@
>
>nestq_num   = popular_qualifier_num(qn);
>targetq_num = popular_qualifier_num(qnt);
>+        if ( nestq_num >= 0 && targetq_num >= 0 )
>+        {
>+          switch(ReduceTypeTable[targetq_num][nestq_num]) {
>+            case RQ_ASIS:
>+              break;
>+
>+              if (onig_verb_warn != onig_null_warn) {
>+                onig_snprintf_with_pattern(buf, WARN_BUFSIZE, env->enc,
>+                    env->pattern, env->pattern_end,
>+                    (UChar* )"redundant nested repeat operator");
>+                (*onig_verb_warn)((char* )buf);
>+              }
>+              goto warn_exit;
>+              break;
>
>-        switch(ReduceTypeTable[targetq_num][nestq_num]) {
>-        case RQ_ASIS:
>-          break;
>-
>-          if (onig_verb_warn != onig_null_warn) {
>-            onig_snprintf_with_pattern(buf, WARN_BUFSIZE, env->enc,
>-                                 env->pattern, env->pattern_end,
>-                                 (UChar* )"redundant nested repeat
operator");
>-            (*onig_verb_warn)((char* )buf);
>-          }
>-          goto warn_exit;
>-          break;
>-
>-          if (onig_verb_warn != onig_null_warn) {
>-            onig_snprintf_with_pattern(buf, WARN_BUFSIZE, env->enc,
>-                                       env->pattern, env->pattern_end,
>-            (UChar* )"nested repeat operator %s and %s was replaced with
'%s'",
>-            PopularQStr[targetq_num], PopularQStr[nestq_num],
>-            ReduceQStr[ReduceTypeTable[targetq_num][nestq_num]]);
>-            (*onig_verb_warn)((char* )buf);
>+            default:
>+              if (onig_verb_warn != onig_null_warn) {
>+                onig_snprintf_with_pattern(buf, WARN_BUFSIZE, env->enc,
>+                    env->pattern, env->pattern_end,
>+                    (UChar* )"nested repeat operator %s and %s was
replaced
>with '%s'",
>+                    PopularQStr[targetq_num], PopularQStr[nestq_num],
>+                    ReduceQStr[ReduceTypeTable[targetq_num][nestq_num]]);
>+                (*onig_verb_warn)((char* )buf);
>+              }
>+              goto warn_exit;
>+              break;
>}
>-          goto warn_exit;
>-          break;
>}
>}
>
>