Page MenuHomePhabricator

Simplify max query name length check in parse_name()
ClosedPublic

Authored by nakihito on Wed, Mar 11, 23:19.

Details

Reviewers
jasonbcox
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Commits
rABC757f9f4b9cbc: Simplify max query name length check in parse_name()
Summary

Rather than having two redundent checks in two different places, having
one simplier check in one place is easier to maintain and makes the
return site consistent.

Also adjusts the unit test to reflect this change. Before the change, the behavior would appear to be correct, however, the function would return an error because of the label length rather than erroring because of the length of the entire qname.

Test Plan
ninja check-bitcoin-seeder

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

nakihito created this revision.Wed, Mar 11, 23:19
Owners added a reviewer: Restricted Owners Package.Wed, Mar 11, 23:19
Herald added a reviewer: Restricted Project. · View Herald TranscriptWed, Mar 11, 23:19
jasonbcox requested changes to this revision.Thu, Mar 12, 00:06
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
src/seeder/test/dns_tests.cpp
134 ↗(On Diff #16887)

Don't do this. Any following tests cases that might rely on maxLengthQName will be relying on something falsely named. Make a new variable if you must.

This revision now requires changes to proceed.Thu, Mar 12, 00:06
nakihito edited the summary of this revision. (Show Details)Thu, Mar 12, 00:13
nakihito updated this revision to Diff 16890.Thu, Mar 12, 00:19

Created new variable to test an oversized qname rather than overwritting maxLegnthQName. Also edited comments for clarity.

jasonbcox accepted this revision.Thu, Mar 12, 00:22
This revision is now accepted and ready to land.Thu, Mar 12, 00:22
This revision was automatically updated to reflect the committed changes.