Opened 3 years ago

Closed 3 years ago

#18462 closed enhancement (implemented)

Rename tor_dup_addr to make it clear that it returns a string

Reported by: teor Owned by:
Priority: Medium Milestone: Tor: 0.2.9.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: easy intro TorCoreTeam201603
Cc: Actual Points:
Parent ID: Points: small
Reviewer: Sponsor:

Description

We discovered in #18454 that it's easy to write code that assumes that tor_dup_addr duplicates an address, rather than formatting that address as a string and duplicating that string.

Let's change the name of that function to tor_addr_to_str_dup or something.

Child Tickets

Attachments (2)

patch.diff (9.4 KB) - added by icanhasaccount 3 years ago.
Rename tor_dup_addr to tor_addr_to_str_dup
patch.2.diff (8.6 KB) - added by icanhasaccount 3 years ago.
Rename mk2

Download all attachments as: .zip

Change History (8)

Changed 3 years ago by icanhasaccount

Attachment: patch.diff added

Rename tor_dup_addr to tor_addr_to_str_dup

comment:1 Changed 3 years ago by icanhasaccount

Attached patch renaming tor_dup_addr to tor_addr_to_str_dup.

comment:2 Changed 3 years ago by teor

Status: newneeds_revision

Thanks for the patch!

Looks great, and does almost exactly what we need it to do.

But in #18454, we'll remove the tor_dup_addr calls in get_interface_address6_list (we actually want to duplicate the memory, not format a string). So we don't need to change those at all in this patch.

Would you mind updating the patch to leave the two tor_dup_addr calls in get_interface_address6_list as they are?

Changed 3 years ago by icanhasaccount

Attachment: patch.2.diff added

Rename mk2

comment:3 Changed 3 years ago by icanhasaccount

Sure - I've updated the patch

comment:4 Changed 3 years ago by teor

Keywords: TorCoreTeam201603 added
Status: needs_revisionneeds_review

Looks great.
We've reached feature freeze for 0.2.8, so let's get this merged in 0.2.9 in March 2016.

comment:5 Changed 3 years ago by nickm

Points: small

comment:6 Changed 3 years ago by nickm

Resolution: implemented
Status: needs_reviewclosed

Merged to master!

Note: See TracTickets for help on using tickets.