[SNORT-DEVEL] calls to strlen() in loop structures possibly inefficient

[Bill Parker <wp02855 () gmail com>]

Hello All,

    In reviewing netvi-01.04.14, I found some calls which are made
to strlen() in for() loops, in which the call to strlen must be
made each time the loop is executed (tested), rather than assigning
strlen() to a variable, and testing the variable (which only calls
strlen() once).  The patch file for /src/mstring.c is
below:

--- mstring.c.orig      2014-04-09 21:17:03.343585833 -0700
+++ mstring.c   2014-04-09 21:25:00.406218040 -0700
@@ -116,7 +116,7 @@
 {
     size_t cur_tok = 0;  /* current token index into array of strings */
     size_t tok_start;    /* index to start of token */
-    size_t i, j;
+    size_t i, j, sep_len, slen;
     int escaped = 0;
     /* It's rare we'll need more than this even if max_toks is set really
      * high.  Store toks here until finished, then allocate.  If more than
@@ -144,16 +144,18 @@
         sep_chars = whitespace;

     /* Meta char cannot also be a separator char */
-    for (i = 0; i < strlen(sep_chars); i++)
+    sep_len = strlen(sep_chars);
+    for (i = 0; i < sep_len; i++)
     {
         if (sep_chars[i] == meta_char)
             return NULL;
     }

     /* Move past initial separator characters and whitespace */
-    for (i = 0; i < strlen(str); i++)
+    slen = strlen(str);
+    for (i = 0; i < slen; i++)
     {
-        for (j = 0; j < strlen(sep_chars); j++)
+        for (j = 0; j < sep_len; j++)
         {
             if ((str[i] == sep_chars[j]) ||
                 isspace((int)str[i]))
@@ -191,7 +193,8 @@

     /* Mark the beginning of the next tok */
     tok_start = i;
-    for (; i < strlen(str); i++)
+    slen = strlen(str);
+    for (; i < slen; i++)
     {
         if (!escaped)
         {
@@ -204,7 +207,8 @@
             }

             /* See if the current character is a separator */
-            for (j = 0; j < strlen(sep_chars); j++)
+           sep_len = strlen(sep_chars);
+            for (j = 0; j < sep_len; j++)
             {
                 if (str[i] == sep_chars[j])
                     break;
@@ -231,9 +235,12 @@
             cur_tok++;

             /* Move past any more separator characters or whitespace */
-            for (; i < strlen(str); i++)
+           slen = strlen(str);
+           sep_len = strlen(sep_chars);
+
+            for (; i < slen; i++)
             {
-                for (j = 0; j < strlen(sep_chars); j++)
+                for (j = 0; j < sep_len; j++)
                 {
                     if ((str[i] == sep_chars[j]) ||
                         isspace((int)str[i]))
@@ -308,7 +315,8 @@
                 }

                 /* Trim whitespace at end of last tok */
-                for (j = strlen(str); j > tok_start; j--)
+               slen = strlen(str);
+                for (j = slen; j > tok_start; j--)
                 {
                     if (!isspace((int)str[j - 1]))
                         break;
@@ -375,7 +383,7 @@
 /* Will not return NULL.  SnortAlloc will fatal if it fails */
 static char * mSplitAddTok(const char *str, const int len, const char
*sep_chars, const char meta_char)
 {
-    size_t i, j, k;
+    size_t i, j, k, sep_len;
     char *tok;
     int tok_len = 0;
     int got_meta = 0;
@@ -396,7 +404,8 @@
         else
         {
             /* See if the current character is a separator */
-            for (j = 0; j < strlen(sep_chars); j++)
+           sep_len = strlen(sep_chars);
+            for (j = 0; j < sep_len; j++)
             {
                 if (str[i] == sep_chars[j])
                     break;
@@ -428,7 +437,8 @@
         else
         {
             /* See if the current character is a separator */
-            for (j = 0; j < strlen(sep_chars); j++)
+           sep_len = strlen(sep_chars);
+            for (j = 0; j < sep_len; j++)
             {
                 if (str[i] == sep_chars[j])
                     break;

In /src/parser.c, the patch file is below:

--- parser.c.orig       2014-04-09 21:32:11.925061866 -0700
+++ parser.c    2014-04-09 21:33:19.424640716 -0700
@@ -288,7 +288,7 @@
      * are comment characters or empty */
     while ((fgets(buf, MAX_LINE_LENGTH, file)) != NULL)
     {
-        int i;
+        int i, idxlen;
         char *index;
         char *ret_line;

@@ -307,7 +307,8 @@
             continue;

         /* Trim off any whitespace at the end of the line */
-        for (i = strlen(index); i > 0; i--)
+       idxlen = strlen(index);
+        for (i = idxlen; i > 0; i--)
         {
             if (!isspace((int)index[i - 1]))
                 break;


A call to 'make' results in a clean compile.

I am attaching the patch file(s) to this email.

Bill Parker (wp02855 at gmail dot com)

Attachment: netvi_parser.c.patch
Description:

Attachment: netvi_mstring.c.patch
Description:

------------------------------------------------------------------------------
Put Bad Developers to Shame
Dominate Development with Jenkins Continuous Integration
Continuously Automate Build, Test & Deployment 
Start a new project now. Try Jenkins in the cloud.
http://p.sf.net/sfu/13600_Cloudbees

_______________________________________________
Snort-devel mailing list
Snort-devel () lists sourceforge net
https://lists.sourceforge.net/lists/listinfo/snort-devel
Archive:
http://sourceforge.net/mailarchive/forum.php?forum_name=snort-devel

Please visit http://blog.snort.org for the latest news about Snort!