bisdn / basebox

A tiny OpenFlow controller for OF-DPA switches.
Mozilla Public License 2.0
45 stars 9 forks source link

port_manager: make destructor virtual #375

Closed rubensfig closed 2 years ago

rubensfig commented 2 years ago

The port_manager is an interface class. The destructor should be implemented by whatever class inherits port_manager.

Avoids the compilation warning

```[3/18] Compiling C++ object baseboxd.p/src_netlink_ctapdev.cc.o
In file included from ../src/netlink/tap_manager.h:13,
from ../src/netlink/ctapdev.cc:18:
../src/netlink/port_manager.h:31:7: warning: 'class
basebox::port_manager' has virtual functions and accessible non-virtual
destructor [-Wnon-virtual-dtor]
31 | class port_manager {
|       ^~~~~~~~~~~~

Signed-off-by: Rubens Figueiredo rubens.figueiredo@bisdn.de

Description

Motivation and Context

How Has This Been Tested?

ideaship commented 2 years ago

The change looks correct, but I failed to reproduce the warning you saw. When I bitbake baseboxd, my log.do_compile does not contain such a warning.

KanjiMonster commented 2 years ago

The port_manager is an interface class. The destructor should be implemented by whatever class inherits port_manager.

After looking into it, that explanation is a bit misleading, since port_manager provides a (default) destructor even after tagging it virtual.

The issue with the non-virtual destructor is that implementing classes cannot override the destructor if their destructor needs to do more, and you are destroying it via an interface typed pointer.

So having

port_manager *pm = new tap_manager();
delete pm;

would currently only call the (empty) port_manager destructor, but when tagging it as virtual, it would instead call the tap_manager's destructor.

ideaship commented 2 years ago

Rephrase/fix this part of the commit message: this interface cannot not override the destructor

rubensfig commented 2 years ago

Updated the commit message

KanjiMonster commented 2 years ago

and the error is raised.

what error? You only show a warning, not an error. (I think you can just drop this half sentence).

Also you have one ``` to many at the beginning of your quoted error message.

rubensfig commented 2 years ago

Again updated the commit message