Iterating over ldap search result set

Question

Im using libldap to obtain information from an OpenLDAP Server. The code is working fine so far but im a bit unsure about how to design the processing of the result set. Lets say the search returns 2 attributes that are both multivalued and can have a lot of values (e.g. memberOf attribute).

For now i made the code as compact as possible:

for (ldap_result = ldap_first_message(ldap_handle, ldap_result); ldap_result != NULL; ldap_result = ldap_next_message(ldap_handle, ldap_result)) {
    switch (msgtype = ldap_msgtype(ldap_result)) {
        case LDAP_RES_SEARCH_ENTRY:
            {
                /* iterate over all requested attributes */
                for (attr = ldap_first_attribute(ldap_handle, ldap_result, &ber); attr != NULL; attr = ldap_next_attribute(ldap_handle, ldap_result, ber)) {
                    /* iterate over attribute values */
                    if ((bvals = ldap_get_values_len(ldap_handle, ldap_result, attr)) != NULL) {
                        int i;
                        for (i = 0; bvals[i] != '\0'; i++) {
                            char *value = bvals[i]->bv_val;
                            ber_len_t len = bvals[i]->bv_len;

                            // if attribute == x
                            if (strcmp(attr, "x")) == 0) {
                                // do_x();
                                // do_y();

                            // if attribute == y
                            } else if (strcmp(attr, "y") == 0) {
                                // do_sth_different();
                            }
                            /* free values structure after each iteration */
                            ldap_value_free_len(bvals);
                        }
                    }
                }
                /* free attributes structure */
                ber_free(ber, 0);
                break;
            }
        default:
            {
            // whatever
            break;
            }
    }
}

So what is happening here? Im getting the first attribute that has for example 100 values. For every value I do a string comparison if its attribute a or b. On modern hardware and less values it wont be a problem but could be on more values. Code is compact but efficiency could be better.

Lets see how we can make it different:

for (ldap_result = ldap_first_message(ldap_handle, ldap_result); ldap_result != NULL; ldap_result = ldap_next_message(ldap_handle, ldap_result)) {
    switch (msgtype = ldap_msgtype(ldap_result)) {
        case LDAP_RES_SEARCH_ENTRY:
            {
                /* iterate over all requested attributes */
                for (attr = ldap_first_attribute(ldap_handle, ldap_result, &ber); attr != NULL; attr = ldap_next_attribute(ldap_handle, ldap_result, ber)) {
                    // if attribute == x
                    if (strcmp(attr, "x")) == 0) {
                        /* iterate over attribute values */
                        if ((bvals = ldap_get_values_len(ldap_handle, ldap_result, attr)) != NULL) {
                            int i;
                            for (i = 0; bvals[i] != '\0'; i++) {
                                /* iterate over attribute values */
                                char *value = bvals[i]->bv_val;
                                ber_len_t len = bvals[i]->bv_len;

                                // do_x();
                                // do_y();
                                /* free values structure after each iteration */
                                ldap_value_free_len(bvals);
                            }
                            /* free attributes structure */
                            ber_free(ber, 0);
                        }
                    // if attribute == y
                    } else if (strcmp(attr, "y") == 0) {
                        /* iterate over attribute values */
                        if ((bvals = ldap_get_values_len(ldap_handle, ldap_result, attr)) != NULL) {
                            int i;
                            for (i = 0; bvals[i] != '\0'; i++) {
                                /* iterate over attribute values */
                                char *value = bvals[i]->bv_val;
                                ber_len_t len = bvals[i]->bv_len;

                                // do_sth_different();
                                /* free values structure after each iteration */
                                ldap_value_free_len(bvals);
                            }
                            /* free attributes structure */
                            ber_free(ber, 0);
                        }
                    }
                }   
            }
        default:
            {
            // whatever
            break;
            }
    }
}

Now there is only one string comparison if its attribute a or b. Unfortunately there is a lot of code duplication for both attributes.

Any recommendations which solution fits better or is there maybe one that i haven't seen yet?

Answer 1

In the first example, save the value of whether attr is x or y so you compute it once without getting the code repetition issue

 for (attr = ldap_first_attribute(ldap_handle, ldap_result, &ber); attr != NULL; attr = ldap_next_attribute(ldap_handle, ldap_result, ber)) {
                /* iterate over attribute values */
      bool isX = strcmp(attr, "x")) == 0;
      bool isY =  strcmp(attr, "y")) == 0;

      ....

      // if attribute == x
      if (isX) {
          // do_x();
          // do_y();

       // if attribute == y
      } else if (isY) {
           // do_sth_different();
       }