Change most while loops over TR_LISTs to for loops
authorJennifer Richards <jennifer@painless-security.com>
Wed, 25 Apr 2018 16:04:37 +0000 (12:04 -0400)
committerJennifer Richards <jennifer@painless-security.com>
Wed, 25 Apr 2018 16:04:37 +0000 (12:04 -0400)
The while loop patter (i = first(); while(i){blah; i = next()}) pattern
was error-prone -- too easy to overlook or forget the next() call.
Changed most of these to for loops to make the iteration more apparent.
Added a few comments. No intentional functional changes.

common/tr_comm_encoders.c
common/tr_filter.c
common/tr_filter_encoders.c
common/tr_gss_names.c
common/tr_idp_encoders.c
common/tr_rp_client_encoders.c
trp/trp_ptable_encoders.c

index 2655fe5..dbc5169 100644 (file)
@@ -143,10 +143,10 @@ static json_t *tr_comm_memb_sources_to_json(TR_COMM_MEMB *first_memb)
     goto cleanup;
 
   /* Iterate over all the memberships for this realm/comm pair that come from different origins */
-  memb = tr_comm_memb_iter_first(iter, first_memb);
-  while (memb) {
+  for (memb = tr_comm_memb_iter_first(iter, first_memb);
+       memb != NULL;
+       memb = tr_comm_memb_iter_next(iter)) {
     ARRAY_APPEND_OR_FAIL(jarray, tr_comm_memb_to_json(memb));
-    memb = tr_comm_memb_iter_next(iter);
   }
 
   /* success */
@@ -171,10 +171,12 @@ static json_t *tr_comm_realms_to_json(TR_COMM_TABLE *ctable, TR_NAME *comm_name,
   TR_COMM_MEMB *memb = NULL;
 
   iter = tr_comm_iter_new(NULL);
-  realm = tr_realm_iter_first(iter, ctable, comm_name);
+  realm = NULL;
 
   /* Do not display the full realm json here, only the name and info relevant to the community listing */
-  while(realm) {
+  for (realm = tr_realm_iter_first(iter, ctable, comm_name);
+       realm != NULL;
+       realm = tr_realm_iter_next(iter)) {
     if (realm->role == role) {
       realm_json = json_object();
       OBJECT_SET_OR_FAIL(realm_json, "realm",
@@ -187,7 +189,6 @@ static json_t *tr_comm_realms_to_json(TR_COMM_TABLE *ctable, TR_NAME *comm_name,
       json_array_append_new(jarray, realm_json);
       realm_json = NULL; /* so we don't free this twice during cleanup */
     }
-    realm = tr_realm_iter_next(iter);
   }
 
   /* Success - increment the reference count so return value survives */
@@ -270,15 +271,15 @@ json_t *tr_comm_table_to_json(TR_COMM_TABLE *ctable)
     goto cleanup;
 
   /* Iterate over communities in the table */
-  comm = tr_comm_table_iter_first(iter, ctable);
-  while (comm) {
+  for (comm = tr_comm_table_iter_first(iter, ctable);
+       comm != NULL;
+       comm = tr_comm_table_iter_next(iter)) {
     comm_json = tr_comm_to_json(ctable, comm);
 
     if (comm_json == NULL)
       goto cleanup;
 
     json_array_append_new(ctable_json, comm_json);
-    comm = tr_comm_table_iter_next(iter);
   }
 
   /* succeeded - set the return value and increment the reference count */
index 9e5b4c3..2b94526 100644 (file)
@@ -426,24 +426,24 @@ int tr_filter_apply(TR_FILTER_TARGET *target,
 
   /* Step through filter lines looking for a match. If a line matches, retval
    * will be set to TR_FILTER_MATCH, so stop then. */
-  this_fline = tr_filter_iter_first(filt_iter, filt);
-  while(this_fline) {
+  for (this_fline = tr_filter_iter_first(filt_iter, filt);
+       this_fline != NULL;
+       this_fline = tr_filter_iter_next(filt_iter)) {
     /* Assume we are going to succeed. If any specs fail to match, we'll set
      * this to TR_FILTER_NO_MATCH. */
     retval=TR_FILTER_MATCH;
-    this_fspec = tr_fline_iter_first(fline_iter, this_fline);
-    while(this_fspec) {
+    for (this_fspec = tr_fline_iter_first(fline_iter, this_fline);
+         this_fspec != NULL;
+         this_fspec = tr_fline_iter_next(fline_iter)) {
       if (!tr_fspec_matches(this_fspec, filt->type, target)) {
         retval=TR_FILTER_NO_MATCH; /* set this in case this is the last filter line */
         break; /* give up on this filter line */
       }
-    this_fspec = tr_fline_iter_next(fline_iter);
     }
 
     if (retval==TR_FILTER_MATCH)
       break;
 
-    this_fline = tr_filter_iter_next(filt_iter);
   }
 
   if (retval==TR_FILTER_MATCH) {
@@ -642,8 +642,9 @@ int tr_filter_validate(TR_FILTER *filt)
       return 0; /* if we get here, either TR_FILTER_TYPE_UNKNOWN or an invalid value was found */
   }
   
-  this_fline = tr_filter_iter_first(filt_iter, filt);
-  while(this_fline) {
+  for (this_fline = tr_filter_iter_first(filt_iter, filt);
+       this_fline != NULL;
+       this_fline = tr_filter_iter_next(filt_iter)) {
     /* check that we recognize the action */
     switch(this_fline->action) {
       case TR_FILTER_ACTION_ACCEPT:
@@ -656,8 +657,9 @@ int tr_filter_validate(TR_FILTER *filt)
         return 0;
     }
 
-    this_fspec = tr_fline_iter_first(fline_iter, this_fline);
-    while(this_fspec) {
+    for (this_fspec = tr_fline_iter_first(fline_iter, this_fline);
+         this_fspec != NULL;
+         this_fspec = tr_fline_iter_next(fline_iter)) {
       if (!tr_filter_validate_spec_field(filt->type, this_fspec)) {
         talloc_free(tmp_ctx);
         return 0;
@@ -668,9 +670,7 @@ int tr_filter_validate(TR_FILTER *filt)
         talloc_free(tmp_ctx);
         return 0;
       }
-      this_fspec = tr_fline_iter_next(fline_iter);
     }
-    this_fline = tr_filter_iter_next(filt_iter);
   }
 
   /* We ran the gauntlet. Success! */
index d4d925d..932be9e 100644 (file)
@@ -58,11 +58,18 @@ do {                                           \
 
 typedef json_t *(ITEM_ENCODER_FUNC)(void *);
 
-enum type_to_array {
-  TYPE_TO_ARRAY_FSPEC,
-  TYPE_TO_ARRAY_CONSTRAINT
+enum array_type {
+  ARRAY_TYPE_FSPEC,
+  ARRAY_TYPE_CONSTRAINT
 };
-static json_t *tr_names_to_json_array(void *obj, enum type_to_array type)
+/**
+ * Make an array of matches from a TR_FSPEC or TR_CONSTRAINT
+ *
+ * @param obj
+ * @param type
+ * @return
+ */
+static json_t *tr_names_to_json_array(void *obj, enum array_type type)
 {
   json_t *jarray = json_array();
   json_t *retval = NULL;
@@ -74,22 +81,22 @@ static json_t *tr_names_to_json_array(void *obj, enum type_to_array type)
     goto cleanup;
 
   switch(type) {
-    case TYPE_TO_ARRAY_FSPEC:
+    case ARRAY_TYPE_FSPEC:
       this_match = tr_fspec_iter_first(&fspec_iter, (TR_FSPEC *)obj);
       break;
 
-    case TYPE_TO_ARRAY_CONSTRAINT:
+    case ARRAY_TYPE_CONSTRAINT:
       this_match = tr_constraint_iter_first(&cons_iter, (TR_CONSTRAINT *)obj);
       break;
   }
   while(this_match) {
     ARRAY_APPEND_OR_FAIL(jarray, tr_name_to_json_string(this_match));
     switch(type) {
-      case TYPE_TO_ARRAY_FSPEC:
+      case ARRAY_TYPE_FSPEC:
         this_match = tr_fspec_iter_next(&fspec_iter);
         break;
 
-      case TYPE_TO_ARRAY_CONSTRAINT:
+      case ARRAY_TYPE_CONSTRAINT:
         this_match = tr_constraint_iter_next(&cons_iter);
         break;
     }
@@ -117,7 +124,7 @@ static json_t *tr_fspec_to_json(TR_FSPEC *fspec)
   OBJECT_SET_OR_FAIL(fspec_json, "field",
                      tr_name_to_json_string(fspec->field));
   OBJECT_SET_OR_FAIL(fspec_json, "matches",
-                     tr_names_to_json_array(fspec, TYPE_TO_ARRAY_FSPEC));
+                     tr_names_to_json_array(fspec, ARRAY_TYPE_FSPEC));
 
   /* succeeded - set the return value and increment the reference count */
   retval = fspec_json;
@@ -139,10 +146,10 @@ static json_t *tr_fspecs_to_json_array(TR_FLINE *fline)
   if ((jarray == NULL) || (iter == NULL))
     goto cleanup;
 
-  this_fspec = tr_fline_iter_first(iter, fline);
-  while(this_fspec) {
+  for (this_fspec = tr_fline_iter_first(iter, fline);
+       this_fspec != NULL;
+       this_fspec = tr_fline_iter_next(iter)) {
     ARRAY_APPEND_OR_FAIL(jarray, tr_fspec_to_json(this_fspec));
-    this_fspec = tr_fline_iter_next(iter);
   }
   /* success */
   retval = jarray;
@@ -172,11 +179,11 @@ static json_t *tr_fline_to_json(TR_FLINE *fline)
                      tr_fspecs_to_json_array(fline));
   if (fline->realm_cons) {
     OBJECT_SET_OR_FAIL(fline_json, "realm_constraints",
-                       tr_names_to_json_array(fline->realm_cons, TYPE_TO_ARRAY_CONSTRAINT));
+                       tr_names_to_json_array(fline->realm_cons, ARRAY_TYPE_CONSTRAINT));
   }
   if (fline->domain_cons) {
     OBJECT_SET_OR_FAIL(fline_json, "domain_constraints",
-                       tr_names_to_json_array(fline->domain_cons, TYPE_TO_ARRAY_CONSTRAINT));
+                       tr_names_to_json_array(fline->domain_cons, ARRAY_TYPE_CONSTRAINT));
   }
 
   /* succeeded - set the return value and increment the reference count */
@@ -199,10 +206,10 @@ static json_t *tr_flines_to_json_array(TR_FILTER *filt)
   if ((jarray == NULL) || (iter == NULL))
     goto cleanup;
 
-  this_fline = tr_filter_iter_first(iter, filt);
-  while(this_fline) {
+  for(this_fline = tr_filter_iter_first(iter, filt);
+      this_fline != NULL;
+      this_fline = tr_filter_iter_next(iter)) {
     ARRAY_APPEND_OR_FAIL(jarray, tr_fline_to_json(this_fline));
-    this_fline = tr_filter_iter_next(iter);
   }
   /* success */
   retval = jarray;
index 318f856..249a8b9 100644 (file)
@@ -91,13 +91,13 @@ TR_GSS_NAMES *tr_gss_names_dup(TALLOC_CTX *mem_ctx, TR_GSS_NAMES *orig)
     talloc_free(tmp_ctx);
     return NULL;
   }
-  this = tr_gss_names_iter_first(iter, orig);
-  while (this) {
+  for (this = tr_gss_names_iter_first(iter, orig);
+       this != NULL;
+       this = tr_gss_names_iter_next(iter)) {
     if (tr_gss_names_add(new, tr_dup_name(this)) != 0) {
       talloc_free(tmp_ctx);
       return NULL;
     }
-    this = tr_gss_names_iter_next(iter);
   }
   /* success */
   talloc_steal(mem_ctx, new);
index fec129a..2167ea9 100644 (file)
@@ -148,10 +148,10 @@ static json_t *tr_apcs_to_json(TR_APC *apcs)
   if ((jarray == NULL) || (iter == NULL))
     goto cleanup;
 
-  apc = tr_apc_iter_first(iter, apcs);
-  while (apc) {
+  for (apc = tr_apc_iter_first(iter, apcs);
+       apc != NULL;
+       apc = tr_apc_iter_next(iter)) {
     ARRAY_APPEND_OR_FAIL(jarray, tr_name_to_json_string(tr_apc_get_id(apc)));
-    apc = tr_apc_iter_next(iter);
   }
 
   /* success */
@@ -192,10 +192,10 @@ static json_t *tr_aaa_servers_to_json(TR_AAA_SERVER *aaas)
   if ((jarray == NULL) || (iter == NULL))
     goto cleanup;
 
-  aaa = tr_aaa_server_iter_first(iter, aaas);
-  while (aaa) {
+  for (aaa = tr_aaa_server_iter_first(iter, aaas);
+       aaa != NULL;
+       aaa = tr_aaa_server_iter_next(iter)) {
     ARRAY_APPEND_OR_FAIL(jarray, tr_aaa_server_to_json(aaa));
-    aaa = tr_aaa_server_iter_next(iter);
   }
 
   /* success */
index d0a5cd0..6a7bda9 100644 (file)
@@ -87,10 +87,10 @@ json_t *tr_rp_clients_to_json(TR_RP_CLIENT *rp_clients)
   if ((jarray == NULL) || (iter == NULL))
     goto cleanup;
 
-  rp_client = tr_rp_client_iter_first(iter, rp_clients);
-  while (rp_client) {
+  for (rp_client = tr_rp_client_iter_first(iter, rp_clients);
+       rp_client != NULL;
+       rp_client = tr_rp_client_iter_next(iter)) {
     ARRAY_APPEND_OR_FAIL(jarray, tr_rp_client_to_json(rp_client));
-    rp_client = tr_rp_client_iter_next(iter);
   }
 
   /* succeeded - set the return value and increment the reference count */
index 2ff8c9c..ea97a10 100644 (file)
@@ -60,10 +60,12 @@ json_t *trp_ptable_to_json(TRP_PTABLE *ptbl)
 {
   TRP_PTABLE_ITER *iter = trp_ptable_iter_new(NULL);
   json_t *ptbl_json = json_array();
-  TRP_PEER *peer = trp_ptable_iter_first(iter, ptbl);
-  while(peer) {
+  TRP_PEER *peer = NULL;
+
+  for (trp_ptable_iter_first(iter, ptbl);
+       peer != NULL;
+       peer = trp_ptable_iter_next(iter)) {
     json_array_append_new(ptbl_json, trp_peer_to_json(peer));
-    peer = trp_ptable_iter_next(iter);
   }
   trp_ptable_iter_free(iter);
   return ptbl_json;