pdns icon indicating copy to clipboard operation
pdns copied to clipboard
verified publisher icon PowerDNS

ComboAddress needs to be refactored

Open rgacogne opened this issue 2 years ago • 3 comments

  • Program: Authoritative, Recursor, dnsdist
  • Issue type: refactoring needed

Short description

Our ComboAddress object is a union of IPv4 and IPv6 structures. This does not fly well with clang-tidy's cppcoreguidelines-pro-type-union-access which has a very strong opinion on unions. As discussed in https://github.com/PowerDNS/pdns/pull/12726#issuecomment-1534880862, we have several options to get rid of the union, but the ComboAddress object is used in a lot of places and completely exposes its internal fields, so the refactoring is going to be a huge task that will conflict badly with work happening in parallel.

rgacogne avatar May 12 '23 09:05 rgacogne

Existing code also assumes the sin_family (and maybe also sin_port) of sockaddr_in and sockaddr_in6 overlap, which adds extra complexity when refactoring this.

omoerbeek avatar May 12 '23 09:05 omoerbeek

sin_port as well, yes. I think our best bet is to stop exposing the internal fields and provide proper accessors. It's going to take a fair amount of work, though. We should probably agree on the interface first.

rgacogne avatar May 12 '23 09:05 rgacogne

Reminder to re-enable the corresponding check once this has been fixed: https://github.com/PowerDNS/pdns/pull/12812

rgacogne avatar May 12 '23 13:05 rgacogne