Opened 5 months ago

Closed 5 months ago

#22213 closed enhancement (fixed)

ROUTER_ADDED_NOTIFY_GENERATOR unused and can be removed

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

Description

As far as I can tell, nothing actually returns ROUTER_ADDED_NOTIFY_GENERATOR.

I think that means we can just get rid of it?

Child Tickets

Change History (4)

comment:1 Changed 5 months ago by nickm

We can, unless we think we're likely to want it in the future for some reason.

comment:2 in reply to:  1 Changed 5 months ago by arma

Replying to nickm:

unless we think we're likely to want it in the future for some reason.

I think we're not likely to use it. In the NOTIFY_GENERATOR case, we do:

    if (r == ROUTER_ADDED_NOTIFY_GENERATOR) {
      /* Accepted with a message. */
      [...]
      write_http_status_line(conn, 400, msg);

but in the "else" case, we do:

    } else {
      [...]
      write_http_status_line(conn, 400, msg);

So, we already have (and are using) a way to send a 400 response along with a message.

The comment where it says we accepted the descriptor seems to be at odds with the 400 response code. In conclusion, I think this is a halfbaked mistake from many years ago. :)

comment:3 Changed 5 months ago by arma

Status: newneeds_review

My cleanup22213 branch does this.

comment:4 Changed 5 months ago by nickm

Milestone: Tor: 0.3.2.x-finalTor: 0.3.1.x-final
Resolution: fixed
Status: needs_reviewclosed

Applied

Note: See TracTickets for help on using tickets.