device: don't let subclasses call NMDevice's act_stage1_prepare()

NMDevice's act_stage1_prepare() now does nothing. Calling it is not
useful and has no effect.

In general, when a subclass overwrites a virtual function, it must be
defined whether the subclass must, may or must-not call the parents
implementation. Likewise, it must be clear when the parents
implementation should be chained: first, as last, does it matter?
In any case, that very much depends on how the parent is implemented
and this can only be solved by documentation and common conventions.

It's a forgiving approach to have a parents implementation do nothing,
then the subclass may call it at any time (or not call it at all).
This is especially useful if classes don't know their parent class well.
But in NetworkManager code the relationship between classes are known
at compile time, so every of these classes knows it derives directly
from NMDevice.

This forgingin approach was what NMDevice's act_stage1_prepare() was doing.
However, it also adds lines of code resulting in a different kind of complexity.
So, it's not clear that this forgiving approach is really better. Note
that it also has a (tiny) runtime and code-size overhead.

Change the expectation of how NMDevice's act_stage1_prepare() should be
called: it is no longer implemented, and subclasses *MUST* not chain up.
This commit is contained in:
Thomas Haller 2019-08-22 09:28:39 +02:00
parent cca0c2b56a
commit dc27512184
19 changed files with 13 additions and 109 deletions

View file

@ -233,12 +233,6 @@ update_connection (NMDevice *device, NMConnection *connection)
static NMActStageReturn
act_stage1_prepare (NMDevice *dev, NMDeviceStateReason *out_failure_reason)
{
NMActStageReturn ret;
ret = NM_DEVICE_CLASS (nm_device_6lowpan_parent_class)->act_stage1_prepare (dev, out_failure_reason);
if (ret != NM_ACT_STAGE_RETURN_SUCCESS)
return ret;
if (!nm_device_hw_addr_set_cloned (dev, nm_device_get_applied_connection (dev), FALSE))
return NM_ACT_STAGE_RETURN_FAILURE;
return NM_ACT_STAGE_RETURN_SUCCESS;

View file

@ -343,10 +343,6 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason)
NMDeviceBond *self = NM_DEVICE_BOND (device);
NMActStageReturn ret = NM_ACT_STAGE_RETURN_SUCCESS;
ret = NM_DEVICE_CLASS (nm_device_bond_parent_class)->act_stage1_prepare (device, out_failure_reason);
if (ret != NM_ACT_STAGE_RETURN_SUCCESS)
return ret;
/* Interface must be down to set bond options */
nm_device_take_down (device, TRUE);
if (!apply_bonding_config (self))

View file

@ -497,15 +497,10 @@ bridge_set_vlan_options (NMDevice *device, NMSettingBridge *s_bridge)
static NMActStageReturn
act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason)
{
NMActStageReturn ret;
NMConnection *connection;
NMSetting *s_bridge;
const Option *option;
ret = NM_DEVICE_CLASS (nm_device_bridge_parent_class)->act_stage1_prepare (device, out_failure_reason);
if (ret != NM_ACT_STAGE_RETURN_SUCCESS)
return ret;
connection = nm_device_get_applied_connection (device);
g_return_val_if_fail (connection, NM_ACT_STAGE_RETURN_FAILURE);

View file

@ -119,12 +119,6 @@ create_and_realize (NMDevice *device,
static NMActStageReturn
act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason)
{
NMActStageReturn ret;
ret = NM_DEVICE_CLASS (nm_device_dummy_parent_class)->act_stage1_prepare (device, out_failure_reason);
if (ret != NM_ACT_STAGE_RETURN_SUCCESS)
return ret;
if (!nm_device_hw_addr_set_cloned (device, nm_device_get_applied_connection (device), FALSE))
return NM_ACT_STAGE_RETURN_FAILURE;

View file

@ -861,11 +861,6 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason)
{
NMDeviceEthernet *self = NM_DEVICE_ETHERNET (device);
NMDeviceEthernetPrivate *priv = NM_DEVICE_ETHERNET_GET_PRIVATE (self);
NMActStageReturn ret;
ret = NM_DEVICE_CLASS (nm_device_ethernet_parent_class)->act_stage1_prepare (device, out_failure_reason);
if (ret != NM_ACT_STAGE_RETURN_SUCCESS)
return ret;
link_negotiation_set (device);

View file

@ -76,16 +76,11 @@ static NMActStageReturn
act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason)
{
nm_auto_close int dirfd = -1;
NMActStageReturn ret;
NMSettingInfiniband *s_infiniband;
char ifname_verified[IFNAMSIZ];
const char *transport_mode;
gboolean ok;
ret = NM_DEVICE_CLASS (nm_device_infiniband_parent_class)->act_stage1_prepare (device, out_failure_reason);
if (ret != NM_ACT_STAGE_RETURN_SUCCESS)
return ret;
s_infiniband = nm_device_get_applied_setting (device, NM_TYPE_SETTING_INFINIBAND);
g_return_val_if_fail (s_infiniband, NM_ACT_STAGE_RETURN_FAILURE);

View file

@ -968,12 +968,6 @@ set_property (GObject *object, guint prop_id,
static NMActStageReturn
act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason)
{
NMActStageReturn ret;
ret = NM_DEVICE_CLASS (nm_device_ip_tunnel_parent_class)->act_stage1_prepare (device, out_failure_reason);
if (ret != NM_ACT_STAGE_RETURN_SUCCESS)
return ret;
if (!nm_device_hw_addr_set_cloned (device, nm_device_get_applied_connection (device), FALSE))
return NM_ACT_STAGE_RETURN_FAILURE;

View file

@ -418,12 +418,6 @@ update_connection (NMDevice *device, NMConnection *connection)
static NMActStageReturn
act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason)
{
NMActStageReturn ret;
ret = NM_DEVICE_CLASS (nm_device_macvlan_parent_class)->act_stage1_prepare (device, out_failure_reason);
if (ret != NM_ACT_STAGE_RETURN_SUCCESS)
return ret;
if (!nm_device_hw_addr_set_cloned (device, nm_device_get_applied_connection (device), FALSE))
return NM_ACT_STAGE_RETURN_FAILURE;

View file

@ -351,11 +351,6 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason)
{
NMDeviceTun *self = NM_DEVICE_TUN (device);
NMDeviceTunPrivate *priv = NM_DEVICE_TUN_GET_PRIVATE (self);
NMActStageReturn ret;
ret = NM_DEVICE_CLASS (nm_device_tun_parent_class)->act_stage1_prepare (device, out_failure_reason);
if (ret != NM_ACT_STAGE_RETURN_SUCCESS)
return ret;
if (priv->props.type == IFF_TUN) {
/* Nothing to do for TUN devices */

View file

@ -477,11 +477,6 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason)
{
NMDevice *parent_device;
NMSettingVlan *s_vlan;
NMActStageReturn ret;
ret = NM_DEVICE_CLASS (nm_device_vlan_parent_class)->act_stage1_prepare (device, out_failure_reason);
if (ret != NM_ACT_STAGE_RETURN_SUCCESS)
return ret;
if (!nm_device_hw_addr_set_cloned (device, nm_device_get_applied_connection (device), FALSE))
return NM_ACT_STAGE_RETURN_FAILURE;

View file

@ -484,12 +484,6 @@ update_connection (NMDevice *device, NMConnection *connection)
static NMActStageReturn
act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason)
{
NMActStageReturn ret;
ret = NM_DEVICE_CLASS (nm_device_vxlan_parent_class)->act_stage1_prepare (device, out_failure_reason);
if (ret != NM_ACT_STAGE_RETURN_SUCCESS)
return ret;
if (!nm_device_hw_addr_set_cloned (device, nm_device_get_applied_connection (device), FALSE))
return NM_ACT_STAGE_RETURN_FAILURE;

View file

@ -131,10 +131,6 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason)
NMDevice *lowpan_device = NULL;
NMActStageReturn ret = NM_ACT_STAGE_RETURN_FAILURE;
ret = NM_DEVICE_CLASS (nm_device_wpan_parent_class)->act_stage1_prepare (device, out_failure_reason);
if (ret != NM_ACT_STAGE_RETURN_SUCCESS)
return ret;
platform = nm_device_get_platform (device);
nm_assert (NM_IS_PLATFORM (platform));

View file

@ -6449,12 +6449,6 @@ sriov_params_cb (GError *error, gpointer data)
nm_device_activate_schedule_stage1_device_prepare (self);
}
static NMActStageReturn
act_stage1_prepare (NMDevice *self, NMDeviceStateReason *out_failure_reason)
{
return NM_ACT_STAGE_RETURN_SUCCESS;
}
/*
* activate_stage1_device_prepare
*
@ -6540,17 +6534,21 @@ activate_stage1_device_prepare (NMDevice *self)
/* Assumed connections were already set up outside NetworkManager */
if (!nm_device_sys_iface_state_is_external_or_assume (self)) {
NMDeviceStateReason failure_reason = NM_DEVICE_STATE_REASON_NONE;
NMDeviceClass *klass = NM_DEVICE_GET_CLASS (self);
ret = NM_DEVICE_GET_CLASS (self)->act_stage1_prepare (self, &failure_reason);
if (ret == NM_ACT_STAGE_RETURN_FAILURE) {
nm_device_state_changed (self, NM_DEVICE_STATE_FAILED, failure_reason);
return;
if (klass->act_stage1_prepare) {
NMDeviceStateReason failure_reason = NM_DEVICE_STATE_REASON_NONE;
ret = klass->act_stage1_prepare (self, &failure_reason);
if (ret == NM_ACT_STAGE_RETURN_FAILURE) {
nm_device_state_changed (self, NM_DEVICE_STATE_FAILED, failure_reason);
return;
}
if (ret == NM_ACT_STAGE_RETURN_POSTPONE)
return;
nm_assert (ret == NM_ACT_STAGE_RETURN_SUCCESS);
}
if (ret == NM_ACT_STAGE_RETURN_POSTPONE)
return;
nm_assert (ret == NM_ACT_STAGE_RETURN_SUCCESS);
}
nm_device_activate_schedule_stage2_device_config (self);
@ -17179,7 +17177,6 @@ nm_device_class_init (NMDeviceClass *klass)
klass->link_changed = link_changed;
klass->is_available = is_available;
klass->act_stage1_prepare = act_stage1_prepare;
klass->act_stage2_config = act_stage2_config;
klass->act_stage3_ip_config_start = act_stage3_ip_config_start;
klass->act_stage4_ip_config_timeout = act_stage4_ip_config_timeout;

View file

@ -637,16 +637,11 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason)
{
NMDeviceTeam *self = NM_DEVICE_TEAM (device);
NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (self);
NMActStageReturn ret = NM_ACT_STAGE_RETURN_SUCCESS;
gs_free_error GError *error = NULL;
NMSettingTeam *s_team;
NMConnection *connection;
const char *cfg;
ret = NM_DEVICE_CLASS (nm_device_team_parent_class)->act_stage1_prepare (device, out_failure_reason);
if (ret != NM_ACT_STAGE_RETURN_SUCCESS)
return ret;
connection = nm_device_get_applied_connection (device);
g_return_val_if_fail (connection, NM_ACT_STAGE_RETURN_FAILURE);

View file

@ -1714,7 +1714,6 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason)
{
NMDeviceIwd *self = NM_DEVICE_IWD (device);
NMDeviceIwdPrivate *priv = NM_DEVICE_IWD_GET_PRIVATE (self);
NMActStageReturn ret;
NMWifiAP *ap = NULL;
gs_unref_object NMWifiAP *ap_fake = NULL;
NMActRequest *req;
@ -1723,10 +1722,6 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason)
const char *mode;
const char *ap_path;
ret = NM_DEVICE_CLASS (nm_device_iwd_parent_class)->act_stage1_prepare (device, out_failure_reason);
if (ret != NM_ACT_STAGE_RETURN_SUCCESS)
return ret;
req = nm_device_get_act_request (device);
g_return_val_if_fail (req, NM_ACT_STAGE_RETURN_FAILURE);

View file

@ -145,13 +145,8 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason)
{
NMDeviceOlpcMesh *self = NM_DEVICE_OLPC_MESH (device);
NMDeviceOlpcMeshPrivate *priv = NM_DEVICE_OLPC_MESH_GET_PRIVATE (self);
NMActStageReturn ret;
gboolean scanning;
ret = NM_DEVICE_CLASS (nm_device_olpc_mesh_parent_class)->act_stage1_prepare (device, out_failure_reason);
if (ret != NM_ACT_STAGE_RETURN_SUCCESS)
return ret;
/* disconnect companion device, if it is connected */
if (nm_device_get_act_request (NM_DEVICE (priv->companion))) {
_LOGI (LOGD_OLPC, "disconnecting companion device %s",

View file

@ -368,15 +368,10 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason)
{
NMDeviceWifiP2P *self = NM_DEVICE_WIFI_P2P (device);
NMDeviceWifiP2PPrivate *priv = NM_DEVICE_WIFI_P2P_GET_PRIVATE (self);
NMActStageReturn ret;
NMConnection *connection;
NMSettingWifiP2P *s_wifi_p2p;
NMWifiP2PPeer *peer;
ret = NM_DEVICE_CLASS (nm_device_wifi_p2p_parent_class)->act_stage1_prepare (device, out_failure_reason);
if (ret != NM_ACT_STAGE_RETURN_SUCCESS)
return ret;
if (!priv->mgmt_iface) {
NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_SUPPLICANT_FAILED);
return NM_ACT_STAGE_RETURN_FAILURE;

View file

@ -2621,7 +2621,6 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason)
{
NMDeviceWifi *self = NM_DEVICE_WIFI (device);
NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self);
NMActStageReturn ret;
NMWifiAP *ap = NULL;
gs_unref_object NMWifiAP *ap_fake = NULL;
NMActRequest *req;
@ -2630,10 +2629,6 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason)
const char *mode;
const char *ap_path;
ret = NM_DEVICE_CLASS (nm_device_wifi_parent_class)->act_stage1_prepare (device, out_failure_reason);
if (ret != NM_ACT_STAGE_RETURN_SUCCESS)
return ret;
req = nm_device_get_act_request (NM_DEVICE (self));
g_return_val_if_fail (req, NM_ACT_STAGE_RETURN_FAILURE);

View file

@ -583,13 +583,8 @@ deactivate_async (NMDevice *self,
static NMActStageReturn
act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason)
{
NMActStageReturn ret;
NMActRequest *req;
ret = NM_DEVICE_CLASS (nm_device_modem_parent_class)->act_stage1_prepare (device, out_failure_reason);
if (ret != NM_ACT_STAGE_RETURN_SUCCESS)
return ret;
req = nm_device_get_act_request (device);
g_return_val_if_fail (req, NM_ACT_STAGE_RETURN_FAILURE);