Re: [PATCH 1/1] device: cache pointer to private-data in NMDevice structure

[Dan Williams <dcbw redhat com>]

On Mon, 2015-12-07 at 17:05 +0100, Thomas Haller wrote:
> We often lookup the private data and retrieve it via
> NM_DEVICE_GET_PRIVATE(),
> which in turn calls G_TYPE_INSTANCE_GET_PRIVATE().
>
> Instead cache the pointer to the private data.
>
> There are up- and downsides:
>
>  - requries additional sizeof(gpointer) bytes for each NMDevice.
>  + retrieving the private pointer will be slightly faster.
>
> But most importantly, the allows to change our common pattern
> to first cache the private data in a variable @priv.
> That is often cumbersome to write, especially for short functions:
> This change gives us a choice to use self->priv directly.
>
> Such a change should not be aimed for every class. Instead it makes
> more sense for NMDevice, where the change pays off better due to the
> class' size and ubiquitous use.

Works for me.

Dan

> ---
>  src/devices/nm-device.c | 10 +++++++---
>  src/devices/nm-device.h |  5 +++++
>  2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
> index cf59a5a..c700a10 100644
> --- a/src/devices/nm-device.c
> +++ b/src/devices/nm-device.c
> @@ -78,7 +78,7 @@ static void nm_device_start_ip_check (NMDevice
> *self);
>  
>  G_DEFINE_ABSTRACT_TYPE (NMDevice, nm_device,
> NM_TYPE_EXPORTED_OBJECT)
>  
> -#define NM_DEVICE_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o),
> NM_TYPE_DEVICE, NMDevicePrivate))
> +#define NM_DEVICE_GET_PRIVATE(o) ((o)->priv)
>  
>  enum {
>       STATE_CHANGED,
> @@ -190,7 +190,7 @@ typedef struct {
>       int ifindex;
>  } DeleteOnDeactivateData;
>  
> -typedef struct {
> +typedef struct _NMDevicePrivate {
>       gboolean in_state_changed;
>       gboolean initialized;
>       gboolean platform_link_initialized;
> @@ -10033,7 +10033,11 @@ _activation_func_to_string
> (ActivationHandleFunc func)
>  static void
>  nm_device_init (NMDevice *self)
>  {
> -     NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
> +     NMDevicePrivate *priv;
> +
> +     priv = G_TYPE_INSTANCE_GET_PRIVATE (self, NM_TYPE_DEVICE,
> NMDevicePrivate);
> +
> +     self->priv = priv;
>  
>       priv->type = NM_DEVICE_TYPE_UNKNOWN;
>       priv->capabilities = NM_DEVICE_CAP_NM_SUPPORTED;
> diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h
> index 5d8104b..45c7306 100644
> --- a/src/devices/nm-device.h
> +++ b/src/devices/nm-device.h
> @@ -110,8 +110,13 @@ typedef enum { /*< skip >*/
>       NM_DEVICE_CHECK_CON_AVAILABLE_ALL                           
>         = (((__NM_DEVICE_CHECK_CON_AVAILABLE_ALL - 1) << 1) - 1),
>  } NMDeviceCheckConAvailableFlags;
>  
> +struct _NMDevicePrivate;
> +
>  struct _NMDevice {
>       NMExportedObject parent;
> +
> +     /* private */
> +     struct _NMDevicePrivate *priv;
>  };
>  
>  /* The flags have an relaxing meaning, that means, specifying more
> flags, can make