Page MenuHomePhabricator

Replace integer literals in dnshandle() with an enum class
ClosedPublic

Authored by nakihito on Dec 6 2019, 18:36.

Details

Summary

The error values used in seeder/dns.cpp are defined in RFC1035 under
RCODE. The integer literals currently used as error values can be
replaced with an enum class with accurate names in line with RFC1035.

Test Plan
make check

Diff Detail

Repository
rABC Bitcoin ABC
Branch
DNSErrorConsts
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 8536
Build 15066: Default Diff Build & Tests
Build 15065: arc lint + arc unit

Event Timeline

Owners added a reviewer: Restricted Owners Package.Dec 6 2019, 18:36
nakihito requested review of this revision.Dec 6 2019, 19:00

Changed constant ints to an enum.

nakihito retitled this revision from Replace integer literals in dnshandle() with constants to Replace integer literals in dnshandle() with an enum.Dec 6 2019, 23:54
deadalnix requested changes to this revision.Dec 8 2019, 01:57
deadalnix added inline comments.
src/seeder/dns.h
18 ↗(On Diff #14685)

Why is this part of the public API, why does this have smurf naming and why is no_error an error?

This revision now requires changes to proceed.Dec 8 2019, 01:57

Renamed variable error -> responseCode, fixed smurf naming, and moved enum to dns.cpp.

nakihito requested review of this revision.Dec 9 2019, 19:47
deadalnix requested changes to this revision.Dec 10 2019, 03:37
deadalnix added inline comments.
src/seeder/dns.cpp
56 ↗(On Diff #14705)

You can make that an enum class so it is strongly typed and names are scoped.

Also DNSResponseCode for types.

This revision now requires changes to proceed.Dec 10 2019, 03:37

Changed enum to enum class.

nakihito retitled this revision from Replace integer literals in dnshandle() with an enum to Replace integer literals in dnshandle() with an enum class.
nakihito edited the summary of this revision. (Show Details)
deadalnix requested changes to this revision.Dec 11 2019, 02:29
deadalnix added inline comments.
src/seeder/dns.cpp
57 ↗(On Diff #14728)

OK or something. NO_ERROR is kind of a bizarre response code.

572 ↗(On Diff #14728)

Just use a constructor and not a cast.

This revision now requires changes to proceed.Dec 11 2019, 02:29

static_cast<uint8_t>() -> uint8_t()

Disucssed with Jason offline, NO_ERROR -> OK.

This revision is now accepted and ready to land.Dec 17 2019, 23:48