> core: refactor private data for NMExportedObject and others

NM_DHCP_MANAGER_GET_PRIVATE ((NMDhcpManager *) object);

Shouldn't the macro be able to cast to NMDhcpManager itself, or at least accept an untyped pointer and do the GObject type validation on it?  I think it's a step backwards to have to do the explicit casting when calling _GET_PRIVATE().

In nm_active_connection_init(), why not just "self->_priv = xxx", why assign to 'priv' first and then self->_priv = priv?

Also, some classes use GET_PRIVATE_PTR and some just GET_PRIVATE, when should each one be used?  I feel like we should just pick one or the other and make them all use it.


Rest generally looks good!

(In reply to Dan Williams from comment #1)
> > core: refactor private data for NMExportedObject and others
> 
> NM_DHCP_MANAGER_GET_PRIVATE ((NMDhcpManager *) object);
> 
> Shouldn't the macro be able to cast to NMDhcpManager itself, or at least
> accept an untyped pointer and do the GObject type validation on it?  I think
> it's a step backwards to have to do the explicit casting when calling
> _GET_PRIVATE().

that is due to how _NM_GET_PRIVATE() is implemented currently.

as it is,
  a) it requires the exact type 
  b) it preserves the const-ness of the self pointer

you question a), yes, that could be different. Note that you cannot do b) without a).
I personally think the "accept any type" is wrong. If you pass a @self pointer of different type, you have to explicitly case. I think behavior a) is desirable.

Well, that is a matter of opinion, of course it could be different.


> In nm_active_connection_init(), why not just "self->_priv = xxx", why assign
> to 'priv' first and then self->_priv = priv?

because @priv is still used below. I think, self->_priv should never be used directly (see nm-modem-broadband.c which is an exception, but it was there before too and I didn't change that).

It's a matter of style... if yes, it could be:

  self->_priv = G_TYPE_INSTANCE_GET_PRIVATE (...
  self->_priv->version_id = _version_id_new ();



> Also, some classes use GET_PRIVATE_PTR and some just GET_PRIVATE, when
> should each one be used?  I feel like we should just pick one or the other
> and make them all use it.

the _NM_GET_PRIVATE() is used for all classes that are "final" (have no subclasses). For example NMActRequest. Note that NMActRequest is an opaque type and the priv pointer is directly embedded.

For types with sub-classes, the type struct must be public, thus, we don't embed the private data directly, but instead have an opaque pointer. 

See:
  git grep -l _NM_GET_PRIVATE_PTR -- src/

Note that _NM_GET_PRIVATE_PTR wastes one additinal pointer per instance. 
E.g. a NMDeviceVeth wastes 3 pointers for NMExportedObject._priv, NMDevice._priv, NMDeviceEthernet._priv.

An alternative would be for non-final types to put the private data in the public header too and just have a code-convention that we do not touch private data. I don't have a strong preference for either, but maybe it's nicer to keep the private data structure hidden.



Now all files in "src" are changed!

LGTM

merged: https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=a5a36d6dfe0b3b4c69d25fae8c70ee07093d3f7b