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

Bill Parker wp02855 at ...2499...
Sat Apr 12 15:58:16 EDT 2014


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)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.snort.org/pipermail/snort-devel/attachments/20140412/bce6fdb7/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: netvi_parser.c.patch
Type: application/octet-stream
Size: 616 bytes
Desc: not available
URL: <https://lists.snort.org/pipermail/snort-devel/attachments/20140412/bce6fdb7/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: netvi_mstring.c.patch
Type: application/octet-stream
Size: 3465 bytes
Desc: not available
URL: <https://lists.snort.org/pipermail/snort-devel/attachments/20140412/bce6fdb7/attachment-0001.obj>


More information about the Snort-devel mailing list