Opened 2 months ago

Closed 3 weeks ago

#31338 closed defect (fixed)

Practracker --list-overbroad produces confusing output when there is an exception

Reported by: teor Owned by: nickm
Priority: Medium Milestone: Tor: 0.4.2.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: 042-should, practracker, easy, network-team-roadmap-july, asn-merge
Cc: nickm, teor Actual Points: 0.1
Parent ID: #29746 Points: 0.1
Reviewer: teor Sponsor: Sponsor31-can

Description

connection_control_process_inbuf() is 115 lines long, but the exceptions file says it should be 113.

This line is spurious and should not appear in the output:

problem function-size /src/feature/control/control.c:connection_control_process_inbuf() 113 -> 0

Ideally, practracker should log a message that there are no over-broad exceptions, but there are violations.

Here is the full output:

$ scripts/maint/practracker/practracker.py --list-overbroad
problem function-size /src/feature/control/control.c:connection_control_process_inbuf() 115
FAILURE: practracker found 1 new problem(s) in the code: see warnings above.

Please fix the problems if you can, and update the exceptions file
(./scripts/maint/practracker/./exceptions.txt) if you can't.

See doc/HACKING/HelpfulTools.md for more information on using practracker.
You can disable this message by setting the TOR_DISABLE_PRACTRACKER environment
variable.

problem function-size /src/feature/control/control.c:connection_control_process_inbuf() 113 -> 0
Exit 1

Child Tickets

Change History (5)

comment:1 Changed 2 months ago by teor

Owner: set to nickm
Status: newassigned

Assigning to nickm because he wrote this code.

comment:2 Changed 4 weeks ago by nickm

See branch ticket31338: it has the amusing PR number https://github.com/torproject/tor/pull/1337 .

I'll put it in needs_review once CI passes.

comment:3 Changed 4 weeks ago by teor

Actual Points: 0.1
Reviewer: teor

Looks fine to me, feel free to put in merge_ready once CI passes.

This patch introduces a double newline, I'm not sure if that was intentional.
If you fix it, feel free to put the result in merge_ready.

comment:4 Changed 4 weeks ago by nickm

Keywords: asn-merge added
Status: assignedmerge_ready

CI has passed. I've removed the double newline and re-pushed.

comment:5 Changed 3 weeks ago by asn

Resolution: fixed
Status: merge_readyclosed

Merged!

Note: See TracTickets for help on using tickets.