Opened 4 years ago

Closed 4 years ago

#17074 closed enhancement (implemented)

Improve coverage on src/common/address.c

Reported by: rjunior Owned by:
Priority: Medium Milestone: Tor: 0.2.8.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: testing, 028-triaged
Cc: Actual Points:
Parent ID: Points: small
Reviewer: Sponsor: SponsorS

Description

The changes are in the branch "address_tests"

https://github.com/twstrike/tor_for_patching/tree/address_tests

Child Tickets

Change History (12)

comment:1 Changed 4 years ago by rjunior

Status: newneeds_review

comment:2 Changed 4 years ago by rl1987

Status: needs_reviewneeds_revision

You are setting a single byte in one address representation, perform a conversion and checking if that single byte is available in another representation. It would be better if you checked if the entire address is the same before and after the conversion.

comment:3 Changed 4 years ago by nickm

Milestone: Tor: 0.2.8.x-final

comment:4 Changed 4 years ago by nickm

Keywords: 028-triaged added

comment:5 Changed 4 years ago by nickm

Sponsor: SponsorS

comment:6 Changed 4 years ago by nickm

Points: small

mark these testing tickets in needs_review as 'small work remaining'

comment:7 Changed 4 years ago by olabini

Hi,

I've now pushed updates to the branch that changes the tests to test the full address.
I've also fixed a few space-issues.

comment:8 Changed 4 years ago by nickm

Status: needs_revisionneeds_review

comment:9 Changed 4 years ago by rl1987

Status: needs_reviewneeds_revision

You should fix the above problem in test_address_tor_addr_to_ipv4n and test_address_tor_addr_to_in as well.

comment:10 Changed 4 years ago by olabini

Hi,

I didn't make the same changes to those two tests because both of them work with s_addr, which represents one ipv4 address as an unsigned long. Thus, those tests actually check the full address, and not just a single byte in the representation.

comment:11 Changed 4 years ago by rl1987

In that case I think it is okay to merge this.

comment:12 Changed 4 years ago by nickm

Resolution: implemented
Severity: Normal
Status: needs_revisionclosed

Merged it!

Note: See TracTickets for help on using tickets.