Rx1513/loop underflows/v4

[catenacyber]

Previous PR: #14804

Contribution style:

• I have read the contributing guide lines at
  https://docs.suricata.io/en/latest/devguide/contributing/contribution-process.html

Our Contribution agreements:

• I have signed the Open Information Security Foundation contribution agreement at
  https://suricata.io/about/contribution-agreement/ (note: this is only required once)

Changes:

• I have created a ticket at
  https://redmine.openinfosecfoundation.org/projects/suricata/issues

Link to ticket: https://redmine.openinfosecfoundation.org/issues/8184

Describe changes:

• Replace decremental "while" loops with incremental "for" loops.

Changes from last PR:

• clang-format
• use for instead of last while

Attaching little reference unit tests, which may be used to inspect difference in generated assembly and results given by changes:
loop_checks.c

[catenacyber]

@jasonish what do you think about the clang-format report ? Looks wrong to us

diff --git a/src/detect.c b/src/detect.c
index beafe60f9c..2f7890516a 100644
--- a/src/detect.c
+++ b/src/detect.c
@@ -892,7 +892,7 @@ next:
         if (break_out_of_packet_filter) {
             break;
         }
-    }
+}

[catenacyber]

@Rx513 can you check what I have done wrong in my changes ?

[Rx1513]

@catenacyber missing continue.

[catenacyber]

What is the hidden effect of continue; before '}' closing the loop ?

[Rx1513]

Turns out:

for (; match_cnt > 0; match_cnt--) {
}

not equivalent to:

while (match_cnt) {
match_cnt--;
}

As in for expression executed after iteration. So either match_cnt should be reduced by one before for or loop expression should be nested.

[catenacyber]

Please do a new PR with a better commit title and this fix :-)

[Rx1513]

Also I think commit title should be changed to something more suitable.

[catenacyber]

Also I think commit title should be changed to something more suitable.

Please do a new version with a better commit title :-)

[jasonish]

@jasonish what do you think about the clang-format report ? Looks wrong to us

diff --git a/src/detect.c b/src/detect.c
index beafe60f9c..2f7890516a 100644
--- a/src/detect.c
+++ b/src/detect.c
@@ -892,7 +892,7 @@ next:
         if (break_out_of_packet_filter) {
             break;
         }
-    }
+}

Yeah, appears the formatter is confused.

[catenacyber]

Yeah, appears the formatter is confused.

So, we should ignore it ?

[jasonish]

Yeah, appears the formatter is confused.

So, we should ignore it ?

Probably, unless someone can investigate it. Should only be an error in this PR, and not continue failing.

[suricata-qa]

ERROR:

ERROR: QA failed on ASAN_TLPR1_suri.

Pipeline = 29809