Opened 3 years ago

Closed 3 years ago

#20692 closed defect (duplicate)

risky duplicate code in rend_config_services()

Reported by: arma Owned by:
Priority: Medium Milestone: Tor: 0.3.0.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: easy
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

In rend_config_services(), we have a loop over options->RendConfigLines, and one of the things it does is:

      if (service) { /* register the one we just finished parsing */
        if (rend_service_check_private_dir(options, service, 0) < 0) {
          rend_service_free(service);
          return -1;
        }

        if (validate_only)
          rend_service_free(service);
        else
          rend_add_service(service);
      }

Then once the loop is finished, it proceeds to call

  if (service) {
    if (rend_service_check_private_dir(options, service, 0) < 0) {
      rend_service_free(service);
      return -1;
    }

    if (validate_only) {
      rend_service_free(service);
    } else {
      rend_add_service(service);
    }
  }

Look familiar? This duplication is going to bite us when somebody changes one part but not the other.

Found while looking at #20638.

Child Tickets

Change History (1)

comment:1 Changed 3 years ago by teor

Resolution: duplicate
Status: newclosed

This is a duplicate of #20484, this issue is fixed in ca21552 of my github branch bug20484_030_v2. It wasn't merged into 0.2.9 because the refactoring was considered non-essential in a late alpha. Feel free to advocate for its inclusion in #20484.

Note: See TracTickets for help on using tickets.