Re: [tor-bugs] #23873 [Core Tor/Tor]: Remove the return value of node_get_prim_orport()

2018-02-16 Thread Tor Bug Tracker & Wiki 
#23873: Remove the return value of node_get_prim_orport()
+
 Reporter:  teor|  Owner:  (none)
 Type:  enhancement | Status:  closed
 Priority:  Medium  |  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor|Version:
 Severity:  Normal  | Resolution:  implemented
 Keywords:  technical-debt  |  Actual Points:
Parent ID:  #24403  | Points:  1
 Reviewer:  |Sponsor:
+
Changes (by nickm):

 * status:  merge_ready => closed
 * resolution:   => implemented


Comment:

 wfm; merging.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23873 [Core Tor/Tor]: Remove the return value of node_get_prim_orport()

2018-02-15 Thread Tor Bug Tracker & Wiki 
#23873: Remove the return value of node_get_prim_orport()
+
 Reporter:  teor|  Owner:  (none)
 Type:  enhancement | Status:  merge_ready
 Priority:  Medium  |  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor|Version:
 Severity:  Normal  | Resolution:
 Keywords:  technical-debt  |  Actual Points:
Parent ID:  #24403  | Points:  1
 Reviewer:  |Sponsor:
+

Comment (by teor):

 This patch looks fine to me.
 We can merge it as long as it compiles and passes the unit tests.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23873 [Core Tor/Tor]: Remove the return value of node_get_prim_orport()

2018-02-08 Thread Tor Bug Tracker & Wiki 
#23873: Remove the return value of node_get_prim_orport()
+
 Reporter:  teor|  Owner:  (none)
 Type:  enhancement | Status:  merge_ready
 Priority:  Medium  |  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor|Version:
 Severity:  Normal  | Resolution:
 Keywords:  technical-debt  |  Actual Points:
Parent ID:  #24403  | Points:  1
 Reviewer:  |Sponsor:
+
Changes (by nickm):

 * status:  needs_review => merge_ready


Comment:

 Thanks, neel!  This patch looks correct to me.  Let's take it early in the
 0.3.4.x series.

 The only change that I would suggest is that the function documentation
 should say that callers should check whether the result is valid before
 using it.  Either you can make that change, or I'll do it when I merge;
 either way is fine with me.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23873 [Core Tor/Tor]: Remove the return value of node_get_prim_orport()

2018-02-06 Thread Tor Bug Tracker & Wiki 
#23873: Remove the return value of node_get_prim_orport()
+
 Reporter:  teor|  Owner:  (none)
 Type:  enhancement | Status:  needs_review
 Priority:  Medium  |  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor|Version:
 Severity:  Normal  | Resolution:
 Keywords:  technical-debt  |  Actual Points:
Parent ID:  #24403  | Points:  1
 Reviewer:  |Sponsor:
+
Changes (by teor):

 * status:  needs_revision => needs_review


Comment:

 Thanks!
 Someone should review and compile this patch in the next few weeks.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23873 [Core Tor/Tor]: Remove the return value of node_get_prim_orport()

2018-02-06 Thread Tor Bug Tracker & Wiki 
#23873: Remove the return value of node_get_prim_orport()
+
 Reporter:  teor|  Owner:  (none)
 Type:  enhancement | Status:  needs_revision
 Priority:  Medium  |  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor|Version:
 Severity:  Normal  | Resolution:
 Keywords:  technical-debt  |  Actual Points:
Parent ID:  #24403  | Points:  1
 Reviewer:  |Sponsor:
+

Comment (by neel):

 I have made the requested changes in the file b23873-p003.diff.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23873 [Core Tor/Tor]: Remove the return value of node_get_prim_orport()

2018-02-06 Thread Tor Bug Tracker & Wiki 
#23873: Remove the return value of node_get_prim_orport()
+
 Reporter:  teor|  Owner:  (none)
 Type:  enhancement | Status:  needs_revision
 Priority:  Medium  |  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor|Version:
 Severity:  Normal  | Resolution:
 Keywords:  technical-debt  |  Actual Points:
Parent ID:  #24403  | Points:  1
 Reviewer:  |Sponsor:
+
Changes (by neel):

 * Attachment "b23873-p003.diff" added.

 Patch to remove the return value of node_get_prim_orport() and
 node_get_prim_dirport() (Revision 3)

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23873 [Core Tor/Tor]: Remove the return value of node_get_prim_orport()

2018-02-05 Thread Tor Bug Tracker & Wiki 
#23873: Remove the return value of node_get_prim_orport()
+
 Reporter:  teor|  Owner:  (none)
 Type:  enhancement | Status:  needs_revision
 Priority:  Medium  |  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor|Version:
 Severity:  Normal  | Resolution:
 Keywords:  technical-debt  |  Actual Points:
Parent ID:  #24403  | Points:  1
 Reviewer:  |Sponsor:
+

Comment (by teor):

 These checks are around the wrong way, they should say
 !tor_addr_port_is_valid(), because we want to use IPv6 when there is no
 valid IPv4:
 {{{
   } else if (node->ipv6_preferred ||
  tor_addr_port_is_valid_ap(_addr, 0)) {
 }}}

 {{{
   } else if (tor_addr_port_is_valid_ap(_addr, 0)
 }}}

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23873 [Core Tor/Tor]: Remove the return value of node_get_prim_orport()

2018-02-05 Thread Tor Bug Tracker & Wiki 
#23873: Remove the return value of node_get_prim_orport()
+
 Reporter:  teor|  Owner:  (none)
 Type:  enhancement | Status:  needs_revision
 Priority:  Medium  |  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor|Version:
 Severity:  Normal  | Resolution:
 Keywords:  technical-debt  |  Actual Points:
Parent ID:  #24403  | Points:  1
 Reviewer:  |Sponsor:
+

Comment (by neel):

 A new patch is available as b23873-p002.diff​.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23873 [Core Tor/Tor]: Remove the return value of node_get_prim_orport()

2018-02-05 Thread Tor Bug Tracker & Wiki 
#23873: Remove the return value of node_get_prim_orport()
+
 Reporter:  teor|  Owner:  (none)
 Type:  enhancement | Status:  needs_revision
 Priority:  Medium  |  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor|Version:
 Severity:  Normal  | Resolution:
 Keywords:  technical-debt  |  Actual Points:
Parent ID:  #24403  | Points:  1
 Reviewer:  |Sponsor:
+
Changes (by neel):

 * Attachment "b23873-p002.diff" added.

 Patch to remove the return value of node_get_prim_orport() and
 node_get_prim_dirport() (Revision 2)

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23873 [Core Tor/Tor]: Remove the return value of node_get_prim_orport()

2018-02-05 Thread Tor Bug Tracker & Wiki 
#23873: Remove the return value of node_get_prim_orport()
+
 Reporter:  teor|  Owner:  (none)
 Type:  enhancement | Status:  needs_revision
 Priority:  Medium  |  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor|Version:
 Severity:  Normal  | Resolution:
 Keywords:  technical-debt  |  Actual Points:
Parent ID:  #24403  | Points:  1
 Reviewer:  |Sponsor:
+
Changes (by teor):

 * status:  new => needs_revision
 * type:  defect => enhancement


Comment:

 Hi, thanks for this patch!

 > The functions that call them need to check the returned address using
 "tor_addr_port_is_valid()". Most callers will pass 0 for "for_listening",
 because they will be connecting to the address, rather than listening on
 it,

 Please revise the patch to use tor_addr_port_is_valid() not
 tor_addr_is_null().

 These checks are around the wrong way, they should say
 `!tor_addr_port_is_valid()`:
 {{{
 } else if (node->ipv6_preferred || !tor_addr_is_null(_addr.addr)) {
 }}}
 {{{
   } else if (!tor_addr_is_null(_addr.addr)
 }}}

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23873 [Core Tor/Tor]: Remove the return value of node_get_prim_orport()

2018-02-05 Thread Tor Bug Tracker & Wiki 
#23873: Remove the return value of node_get_prim_orport()
+
 Reporter:  teor|  Owner:  (none)
 Type:  defect  | Status:  new
 Priority:  Medium  |  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor|Version:
 Severity:  Normal  | Resolution:
 Keywords:  technical-debt  |  Actual Points:
Parent ID:  #24403  | Points:  1
 Reviewer:  |Sponsor:
+
Changes (by neel):

 * Attachment "b23873-p001.diff" added.

 Patch to remove the return value of node_get_prim_orport() and
 node_get_prim_dirport() (Revision 1)

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23873 [Core Tor/Tor]: Remove the return value of node_get_prim_orport()

2018-02-05 Thread Tor Bug Tracker & Wiki 
#23873: Remove the return value of node_get_prim_orport()
+
 Reporter:  teor|  Owner:  (none)
 Type:  defect  | Status:  new
 Priority:  Medium  |  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor|Version:
 Severity:  Normal  | Resolution:
 Keywords:  technical-debt  |  Actual Points:
Parent ID:  #24403  | Points:  1
 Reviewer:  |Sponsor:
+

Comment (by neel):

 I have a patch for this in b23873-p001.diff.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23873 [Core Tor/Tor]: Remove the return value of node_get_prim_orport()

2018-02-05 Thread Tor Bug Tracker & Wiki 
#23873: Remove the return value of node_get_prim_orport()
+
 Reporter:  teor|  Owner:  (none)
 Type:  defect  | Status:  new
 Priority:  Medium  |  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor|Version:
 Severity:  Normal  | Resolution:
 Keywords:  technical-debt  |  Actual Points:
Parent ID:  #24403  | Points:  1
 Reviewer:  |Sponsor:
+
Changes (by neel):

 * cc: neel@… (added)


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23873 [Core Tor/Tor]: Remove the return value of node_get_prim_orport()

2018-02-03 Thread Tor Bug Tracker & Wiki 
#23873: Remove the return value of node_get_prim_orport()
+
 Reporter:  teor|  Owner:  (none)
 Type:  defect  | Status:  new
 Priority:  Medium  |  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor|Version:
 Severity:  Normal  | Resolution:
 Keywords:  technical-debt  |  Actual Points:
Parent ID:  #24403  | Points:  1
 Reviewer:  |Sponsor:
+

Comment (by teor):

 Replying to [comment:5 neel]:
 > In `node_get_prim_orport()`, if a ORPort exists, the return value comes
 from `RETURN_IPV4_AP()`. It seems `RETURN_IPV4_AP()` is a macro which
 returns 0 on succes. However, there is a `node_get_prim_dirport()` which
 also calls `RETURN_IPV4_AP()` for a return value. Should I also modify
 `node_get_prim_dirport()` (remove the return value and modify functions
 depending on it to check for a `NULL` address)?

 Yes, you should modify both the ORPort and DirPort functions.

 The functions that call them need to check the returned address using
 "tor_addr_port_is_valid()". Most callers will pass 0 for "for_listening",
 because they will be connecting to the address, rather than listening on
 it,

 Most of the callers already do a validity check, or they ignore invalid
 addresses later on.
 We should make sure each function handles invalid addresses correctly.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23873 [Core Tor/Tor]: Remove the return value of node_get_prim_orport()

2018-02-03 Thread Tor Bug Tracker & Wiki 
#23873: Remove the return value of node_get_prim_orport()
+
 Reporter:  teor|  Owner:  (none)
 Type:  defect  | Status:  new
 Priority:  Medium  |  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor|Version:
 Severity:  Normal  | Resolution:
 Keywords:  technical-debt  |  Actual Points:
Parent ID:  #24403  | Points:  1
 Reviewer:  |Sponsor:
+

Comment (by neel):

 In `node_get_prim_orport()`, if a ORPort exists, the return value comes
 from `RETURN_IPV4_AP()`. It seems `RETURN_IPV4_AP()` is a macro which
 returns 0 on succes. However, there is a `node_get_prim_dirport()` which
 also calls `RETURN_IPV4_AP()` for a return value. Should I also modify
 `node_get_prim_dirport()` (remove the return value and modify functions
 depending on it to check for a `NULL` address)?

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23873 [Core Tor/Tor]: Remove the return value of node_get_prim_orport() (was: Tor often forgets to check the return value of node_get_prim_orport())

2017-12-24 Thread Tor Bug Tracker & Wiki 
#23873: Remove the return value of node_get_prim_orport()
+
 Reporter:  teor|  Owner:  (none)
 Type:  defect  | Status:  new
 Priority:  Medium  |  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor|Version:
 Severity:  Normal  | Resolution:
 Keywords:  technical-debt  |  Actual Points:
Parent ID:  #24403  | Points:  1
 Reviewer:  |Sponsor:
+

Old description:

> Also, we don't clear the address and port when we fail.
>
> This probably doesn't matter right now, but it matters as soon as we try
> to change node_get_prim_orport().

New description:

 ~~Also, we don't clear the address and port when we fail.~~

 ~~This probably doesn't matter right now, but it matters as soon as we try
 to change node_get_prim_orport().~~

 We don't check it, so let's remove it, and check for the null address
 instead.

--

Comment (by teor):

 #23874 unconditionally clears the address, so all that is left here is to
 remove the return value.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs