switch_event: clarify EF_UNIQ_HEADERS dup comment, brace one-line test loops

Address review feedback on the switch_event_dup change:

- src/switch_event.c: the fast-path comment claimed todup's headers are "already unique" unconditionally. That only holds when EF_UNIQ_HEADERS is set (names are deduped on insert); reword to scope the claim to that case and to the verbatim-copy intent.

- tests/unit/switch_event.c: expand one-line loops to braced form per the SignalWire coding guidelines.

Signed-off-by: Calvin Ellison <cellison@youmail.com>

Author: cellison-hashbrowns

src/switch_event.c

@@ -1346,11 +1346,13 @@ SWITCH_DECLARE(switch_status_t) switch_event_dup(switch_event_t **event, switch_
 	(*event)->bind_user_data = todup->bind_user_data;
 	(*event)->flags = todup->flags;
 
-	/* todup's headers are already unique (uniqueness is enforced on insert for an
-	 * EF_UNIQ_HEADERS event), so the per-add duplicate scan that
-	 * switch_event_base_add_header() performs when EF_UNIQ_HEADERS is set is
-	 * redundant while cloning and makes this loop O(n^2) in the header count.
-	 * Suppress the scan for the copy, then restore the flag. */
+	/* The destination inherited todup's flags above. When EF_UNIQ_HEADERS is set,
+	 * switch_event_base_add_header() rescans the partial header list on every add
+	 * to keep names unique, making this copy loop O(n^2). A clone should reproduce
+	 * todup's list verbatim rather than run a fresh uniqueness pass, and a
+	 * well-formed EF_UNIQ_HEADERS source is already unique (its names were deduped
+	 * on insert), so that scan only adds cost here. Suppress it for the copy, then
+	 * restore the flag so later adds enforce uniqueness again. */
 	if (switch_test_flag((*event), EF_UNIQ_HEADERS)) {
 		switch_clear_flag((*event), EF_UNIQ_HEADERS);
 		restore_uniq_headers = 1;

tests/unit/switch_event.c

@@ -130,14 +130,18 @@ FST_TEST_BEGIN(dup_uniq_bench)
     switch_event_add_header_string(src, SWITCH_STACK_BOTTOM, name, val);
   }
   S = 0;
-  for (hp = src->headers; hp; hp = hp->next) S++;
+  for (hp = src->headers; hp; hp = hp->next) {
+    S++;
+  }
   fst_xcheck(S >= 191, "source has at least the 191 added headers");
 
   /* correctness: dup preserves header count, the uniq flag, and values */
   status = switch_event_dup(&dup, src);
   fst_xcheck(status == SWITCH_STATUS_SUCCESS, "dup ok");
   count = 0;
-  for (hp = dup->headers; hp; hp = hp->next) count++;
+  for (hp = dup->headers; hp; hp = hp->next) {
+    count++;
+  }
   fst_xcheck(count == S, "dup header count equals source");
   fst_xcheck(switch_test_flag(dup, EF_UNIQ_HEADERS) != 0, "dup keeps EF_UNIQ_HEADERS");
   fst_check_string_equals(switch_event_get_header(dup, "var_0"), "value_0_some_padding_payload");
@@ -149,7 +153,11 @@ FST_TEST_BEGIN(dup_uniq_bench)
   status = switch_event_dup(&dup, src);
   fst_xcheck(status == SWITCH_STATUS_SUCCESS, "dup ok (2)");
   k = 0;
-  for (hp = dup->headers; hp; hp = hp->next) { if (!strcmp(hp->name, "var_0")) k++; }
+  for (hp = dup->headers; hp; hp = hp->next) {
+    if (!strcmp(hp->name, "var_0")) {
+      k++;
+    }
+  }
   fst_xcheck(k == 1, "var_0 present exactly once after re-set + dup");
   fst_check_string_equals(switch_event_get_header(dup, "var_0"), "REPLACED");
   switch_event_destroy(&dup);
@@ -201,11 +209,21 @@ FST_TEST_BEGIN(dup_faithful_copy)
   fst_xcheck(switch_test_flag(src, EF_UNIQ_HEADERS) != 0, "CHANNEL_DATA is EF_UNIQ");
   switch_event_add_header_string(src, SWITCH_STACK_BOTTOM, "k", "v1");
   switch_event_add_header_string(src, SWITCH_STACK_BOTTOM, "k", "v2");
-  n = 0; for (hp = src->headers; hp; hp = hp->next) { if (!strcmp(hp->name, "k")) n++; }
+  n = 0;
+  for (hp = src->headers; hp; hp = hp->next) {
+    if (!strcmp(hp->name, "k")) {
+      n++;
+    }
+  }
   fst_xcheck(n == 1, "EF_UNIQ source keeps 'k' unique (collapsed to last)");
   status = switch_event_dup(&dup, src);
   fst_xcheck(status == SWITCH_STATUS_SUCCESS, "dup ok (unique)");
-  n = 0; for (hp = dup->headers; hp; hp = hp->next) { if (!strcmp(hp->name, "k")) n++; }
+  n = 0;
+  for (hp = dup->headers; hp; hp = hp->next) {
+    if (!strcmp(hp->name, "k")) {
+      n++;
+    }
+  }
   fst_xcheck(n == 1, "dup of unique source stays unique");
   fst_check_string_equals(switch_event_get_header(dup, "k"), "v2");
   switch_event_destroy(&dup);
@@ -218,12 +236,22 @@ FST_TEST_BEGIN(dup_faithful_copy)
   switch_event_add_header_string(src, SWITCH_STACK_BOTTOM, "dupkey", "first");
   switch_event_add_header_string(src, SWITCH_STACK_BOTTOM, "dupkey", "second");
   switch_set_flag(src, EF_UNIQ_HEADERS);
-  n = 0; for (hp = src->headers; hp; hp = hp->next) { if (!strcmp(hp->name, "dupkey")) n++; }
+  n = 0;
+  for (hp = src->headers; hp; hp = hp->next) {
+    if (!strcmp(hp->name, "dupkey")) {
+      n++;
+    }
+  }
   fst_xcheck(n == 2, "malformed source holds 2 'dupkey' headers");
   status = switch_event_dup(&dup, src);
   fst_xcheck(status == SWITCH_STATUS_SUCCESS, "dup ok (malformed)");
   fst_xcheck(switch_test_flag(dup, EF_UNIQ_HEADERS) != 0, "dup keeps EF_UNIQ flag");
-  n = 0; for (hp = dup->headers; hp; hp = hp->next) { if (!strcmp(hp->name, "dupkey")) n++; }
+  n = 0;
+  for (hp = dup->headers; hp; hp = hp->next) {
+    if (!strcmp(hp->name, "dupkey")) {
+      n++;
+    }
+  }
   fst_xcheck(n == 2, "dup faithfully preserves both 'dupkey' headers (no silent collapse)");
   switch_event_destroy(&dup);
   switch_event_destroy(&src);