[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #23954 [Core Tor/Tor]: Race condition in LOG_PROTOCOL_WARN



#23954: Race condition in LOG_PROTOCOL_WARN
-----------------------------+------------------------------------
 Reporter:  nickm            |          Owner:  nickm
     Type:  defect           |         Status:  needs_revision
 Priority:  Medium           |      Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor     |        Version:
 Severity:  Normal           |     Resolution:
 Keywords:  review-group-31  |  Actual Points:
Parent ID:                   |         Points:
 Reviewer:  dgoulet          |        Sponsor:
-----------------------------+------------------------------------
Changes (by dgoulet):

 * status:  needs_review => needs_revision
 * reviewer:   => dgoulet


Comment:

 * Nitpick: I would wrap this in its own function like the init and cleanup
 functions:

 {{{
 +  {
 +    int warning_severity = options->ProtocolWarnings ? LOG_WARN :
 LOG_INFO;
 +    atomic_counter_exchange(&protocol_warning_severity_level,
 +                            warning_severity);
 +  }
 }}}

  Like a "set" function that wraps the `exchange()` so we have one function
 that is allow to set `protocol_warning_severity_level` so
 `init_protocol_warning_severity_level()` can also use it.

 * Quick thing also, I would document the reason why we use an atomic
 counter for that value that is at least explaining what the possible race
 is exactly:

 {{{
 +static atomic_counter_t protocol_warning_severity_level;
 }}}

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/23954#comment:4>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs