Re: [tor-bugs] #18402 [Core Tor/Tor]: Reduce duplicate code in parse_*_time functions

["Tor Bug Tracker & Wiki" <blackhole@xxxxxxxxxxxxxx>]

#18402: Reduce duplicate code in parse_*_time functions
-------------------------------------------------+-------------------------
 Reporter:  icanhasaccount                       |          Owner:
     Type:  enhancement                          |         Status:
                                                 |  needs_revision
 Priority:  Low                                  |      Milestone:  Tor:
                                                 |  0.2.???
Component:  Core Tor/Tor                         |        Version:  Tor:
                                                 |  unspecified
 Severity:  Minor                                |     Resolution:
 Keywords:  TorCoreTeam-postponed-201604,        |  Actual Points:
  nickm-deferred-20160905                        |
Parent ID:                                       |         Points:  1
 Reviewer:  yawning                              |        Sponsor:
-------------------------------------------------+-------------------------

Comment (by icanhasaccount):

 Replying to [comment:10 teor]:
 > Replying to [comment:9 icanhasaccount]:
 > > Hey all - I've had another look at this and its more complicated than
 I first thought.
 > >
 > > Looking at the 3 parse_*_time functions in util.c:
 > >
 > > parse_rfc1123_time: called from parse_http_response, or/directory.c
 > > The result of parse_rfc1123_time isn't actually used anywhere in this
 function - could this be removed?
 >
 > parse_http_response is passed a date parameter, which is set to the
 result of parse_rfc1123_time. connection_dir_client_reached_eof uses it,
 but connection_read_https_proxy_response does not.
 >
 > > parse_http_time: called from directory_handle_command_get,
 /or/directory.c
 > > if_modified_since always appears to be zero, so unless logging the
 failure result of parse_http_time is useful it seems this could be removed
 as well?
 >
 > if_modified_since is only set to zero if tor_timegm fails. Otherwise,
 its value is provided by tor_timegm. And it is used by
 directory_handle_command_get.
 >

 Ah yes - you're correct - parse_rfc1123_time and parse_iso_time_ both
 return the result of tor_timegm,
  but parse_http_time doesn't.

 > > parse_iso_time_: called from rend_parse_v2_service_descriptor,
 or/routerparse.c and entry_guards_parse_state_for_guard_selection,
 or/entrynodes.c , both functions use the result of parse_is_time_ so this
 appears fine.
 >
 > And parse_iso_time_ (trailing underscore) is called by parse_iso_time
 (no trailing underscore), which is called by about 20 functions.
 >
 > > So before revising the diff I thought I'd check as a first step if
 parse_rfc1123_time and parse_http_time could (or should) be removed?
 >
 > No, they need to stay.

 Yup - thank you for taking the time to read through my ramblings and for
 providing a detailed explanation.

 I think its probably best to drop the original diff and close the report
 (the original diff isn't really correct either and no longer applies) -
 reducing the duplicated code would be worthwhile but its a task best left
 to folks more familiar with the tor code.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/18402#comment:11>
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