[tor-bugs] #29133 [Core Tor/Tor]: Refactor dirserv_read_measured_bandwidths

[Tor Bug Tracker & Wiki]

#29133: Refactor dirserv_read_measured_bandwidths
-------------------------------------------------+-------------------------
 Reporter:  teor                                 |          Owner:  (none)
     Type:  defect                               |         Status:  new
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  unspecified
Component:  Core Tor/Tor                         |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  technical-debt tor-crypto tor-       |  Actual Points:
  dirauth tor-bwauth 041-proposed                |
Parent ID:  #26698                               |         Points:
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------

Comment (by juga):

 i'm copying them here the notes about refactoring, since the PR comments
 are long.

 Commented in
 https://github.com/torproject/tor/pull/497#discussion_r249245814:

 {{{
 This function keeps on getting bigger. If we just add code, it will get
 harder and harder to change.

 Let's think about splitting up what this function has to do into separate
 functions:

     read a line from the file
     add the line to the hash
     process the line
         check the timestamp
             add it to the headers
             stop accepting timestamps
         check for a k=v header, the terminator, or a relay line
             add headers to the headers
             at the terminator or first relay line, stop accepting k=v
 headers
             add relay lines to the relays
 }}}

 Commented in
 https://github.com/torproject/tor/pull/497#discussion_r249244433:

 Remove line
 {{{
  /* Initialise line, so that we can't possibly run off the end. */
 }}}

 Commented in
 https://github.com/torproject/tor/pull/497#discussion_r249244629:

 {{{
 3. The comment about fgets is outdated: let's update it to say getdelim.

 4. the timestamp check is outdated, if it succeeds, getdelim always NUL-
 terminates its output

     We can delete the strlen() check, and move the newline check.
 }}}

 Commented in
 https://github.com/torproject/tor/pull/497#discussion_r249244743

 {{{
 If we delete case 3 above, we need to move the newline check here:

 -   line[strlen(line)-1] = '\0';
 +   if (line[strlen(line)-1] == '\n')
 +     line[strlen(line)-1] = '\0';
 }}}

 Commented in
 https://github.com/torproject/tor/pull/497#discussion_r249244842:

 {{{
 This EOF check is redundant, getline tells you when the input is done by
 returning <= 0.

   while (!feof(fp)) {
 }}}

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/29133#comment:3>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online