GNOME.org
 

Re: [PATCH] config: convert NMConfig to object

On Tue, 2013-01-15 at 18:48 +0100, Thomas Bechtold wrote:
> ---
>  src/main.c      |   2 +-
>  src/nm-config.c | 209 ++++++++++++++++++++++++++++++++++++++------------------
>  src/nm-config.h |  38 ++++++++---
>  3 files changed, 172 insertions(+), 77 deletions(-)

Looks good except for some whitespace stuff that I cleaned up.  It's now
pushed to the dcbw/config branch awaiting further review.

Thanks!
Dan

> diff --git a/src/main.c b/src/main.c
> index b074bd8..98aae4f 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -693,7 +693,7 @@ done:
>  	if (pidfile && wrote_pidfile)
>  		unlink (pidfile);
>  
> -	nm_config_free (config);
> +	g_object_unref (config);
>  
>  	/* Free options */
>  	g_free (pidfile);
> diff --git a/src/nm-config.c b/src/nm-config.c
> index 6e801d3..9e682d1 100644
> --- a/src/nm-config.c
> +++ b/src/nm-config.c
> @@ -16,6 +16,7 @@
>   * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
>   *
>   * Copyright (C) 2011 Red Hat, Inc.
> + * Copyright (C) 2013 Thomas Bechtold <thomasbechtold jpberlin de>
>   */
>  
>  #include <config.h>
> @@ -27,7 +28,7 @@
>  #define NM_DEFAULT_SYSTEM_CONF_FILE  NMCONFDIR "/NetworkManager.conf"
>  #define NM_OLD_SYSTEM_CONF_FILE      NMCONFDIR "/nm-system-settings.conf"
>  
> -struct NMConfig {
> +typedef struct {
>  	char *path;
>  	char **plugins;
>  	char *dhcp_client;
> @@ -37,7 +38,13 @@ struct NMConfig {
>  	char *connectivity_uri;
>  	guint connectivity_interval;
>  	char *connectivity_response;
> -};
> +} NMConfigPrivate;
> +
> +static NMConfig *singleton = NULL;
> +
> +G_DEFINE_TYPE (NMConfig, nm_config, G_TYPE_OBJECT)
> +
> +#define NM_CONFIG_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), NM_TYPE_CONFIG, NMConfigPrivate))
>  
>  /************************************************************************/
>  
> @@ -52,12 +59,95 @@ nm_config_error_quark (void)
>  
>  /************************************************************************/
>  
> +static void
> +nm_config_init (NMConfig *config)
> +{
> +	NMConfigPrivate *priv = NM_CONFIG_GET_PRIVATE (config);
> +	priv->path = NULL;
> +	priv->plugins = NULL;
> +	priv->dhcp_client = NULL;
> +	priv->dns_plugins = NULL;
> +	priv->log_level = NULL;
> +	priv->log_domains = NULL;
> +	priv->connectivity_uri = NULL;
> +	priv->connectivity_interval = 0;
> +	priv->connectivity_response = NULL;
> +}
> +
> +static void
> +dispose (GObject *object)
> +{
> +	NMConfig *config = NM_CONFIG (object);
> +	NMConfigPrivate *priv = NM_CONFIG_GET_PRIVATE (config);
> +
> +	if (priv->path) {
> +		g_free (priv->path);
> +		priv->path = NULL;
> +	}
> +	if (priv->plugins) {
> +		g_strfreev (priv->plugins);
> +		priv->plugins = NULL;
> +	}
> +	if (priv->dhcp_client) {
> +		g_free (priv->dhcp_client);
> +		priv->dhcp_client = NULL;
> +	}
> +	if (priv->dns_plugins) {
> +		g_strfreev (priv->dns_plugins);
> +		priv->dns_plugins = NULL;
> +	}
> +	if (priv->log_level) {
> +		g_free (priv->log_level);
> +		priv->log_level = NULL;
> +	}
> +	if (priv->log_domains) {
> +		g_free (priv->log_domains);
> +		priv->log_domains = NULL;
> +	}
> +	if (priv->connectivity_uri) {
> +		g_free (priv->connectivity_uri);
> +		priv->connectivity_uri = NULL;
> +	}
> +	if (priv->connectivity_response) {
> +		g_free (priv->connectivity_response);
> +		priv->connectivity_response = NULL;
> +	}
> +
> +	G_OBJECT_CLASS (nm_config_parent_class)->dispose (object);
> +}
> +
> +static void
> +finalize (GObject *gobject)
> +{
> +	singleton = NULL;
> +	G_OBJECT_CLASS (nm_config_parent_class)->finalize (gobject);
> +}
> +
> +
> +static void
> +nm_config_class_init (NMConfigClass *config_class)
> +{
> +	GObjectClass *object_class = G_OBJECT_CLASS (config_class);
> +
> +	g_type_class_add_private (config_class, sizeof (NMConfigPrivate));
> +	object_class->dispose = dispose;
> +	object_class->finalize = finalize;
> +}
> +
> +NMConfig *
> +nm_config_get (void)
> +{
> +	g_assert (singleton);
> +	g_object_ref (singleton);
> +	return singleton;
> +}
> +
>  const char *
>  nm_config_get_path (NMConfig *config)
>  {
>  	g_return_val_if_fail (config != NULL, NULL);
>  
> -	return config->path;
> +	return NM_CONFIG_GET_PRIVATE (config)->path;
>  }
>  
>  const char **
> @@ -65,7 +155,7 @@ nm_config_get_plugins (NMConfig *config)
>  {
>  	g_return_val_if_fail (config != NULL, NULL);
>  
> -	return (const char **) config->plugins;
> +	return (const char **) NM_CONFIG_GET_PRIVATE (config)->plugins;
>  }
>  
>  const char *
> @@ -73,7 +163,7 @@ nm_config_get_dhcp_client (NMConfig *config)
>  {
>  	g_return_val_if_fail (config != NULL, NULL);
>  
> -	return config->dhcp_client;
> +	return NM_CONFIG_GET_PRIVATE (config)->dhcp_client;;
>  }
>  
>  const char **
> @@ -81,7 +171,7 @@ nm_config_get_dns_plugins (NMConfig *config)
>  {
>  	g_return_val_if_fail (config != NULL, NULL);
>  
> -	return (const char **) config->dns_plugins;
> +	return (const char **) NM_CONFIG_GET_PRIVATE (config)->dns_plugins;
>  }
>  
>  const char *
> @@ -89,7 +179,7 @@ nm_config_get_log_level (NMConfig *config)
>  {
>  	g_return_val_if_fail (config != NULL, NULL);
>  
> -	return config->log_level;
> +	return NM_CONFIG_GET_PRIVATE (config)->log_level;;
>  }
>  
>  const char *
> @@ -97,7 +187,7 @@ nm_config_get_log_domains (NMConfig *config)
>  {
>  	g_return_val_if_fail (config != NULL, NULL);
>  
> -	return config->log_domains;
> +	return NM_CONFIG_GET_PRIVATE (config)->log_domains;
>  }
>  
>  const char *
> @@ -105,7 +195,7 @@ nm_config_get_connectivity_uri (NMConfig *config)
>  {
>  	g_return_val_if_fail (config != NULL, NULL);
>  
> -	return config->connectivity_uri;
> +	return NM_CONFIG_GET_PRIVATE (config)->connectivity_uri;
>  }
>  
>  const guint
> @@ -113,7 +203,7 @@ nm_config_get_connectivity_interval (NMConfig *config)
>  {
>  	g_return_val_if_fail (config != NULL, 0);
>  
> -	return config->connectivity_interval;
> +	return NM_CONFIG_GET_PRIVATE (config)->connectivity_interval;
>  }
>  
>  const char *
> @@ -121,7 +211,7 @@ nm_config_get_connectivity_response (NMConfig *config)
>  {
>  	g_return_val_if_fail (config != NULL, NULL);
>  
> -	return config->connectivity_response;
> +	return NM_CONFIG_GET_PRIVATE (config)->connectivity_response;
>  }
>  
> 
> @@ -138,6 +228,7 @@ fill_from_file (NMConfig *config,
>                  const char *cli_connectivity_response,
>                  GError **error)
>  {
> +	NMConfigPrivate *priv = NM_CONFIG_GET_PRIVATE (config);
>  	GKeyFile *kf;
>  	gboolean success = FALSE;
>  
> @@ -155,41 +246,41 @@ fill_from_file (NMConfig *config,
>  
>  	g_key_file_set_list_separator (kf, ',');
>  	if (g_key_file_load_from_file (kf, path, G_KEY_FILE_NONE, error)) {
> -		config->path = g_strdup (path);
> +		priv->path = g_strdup (path);
>  
>  		/* CLI provided options override config file options */
>  		if (cli_plugins && strlen (cli_plugins))
> -			config->plugins = g_strsplit_set (cli_plugins, ",", 0);
> +			priv->plugins = g_strsplit_set (cli_plugins, ",", 0);
>  		else
> -			config->plugins = g_key_file_get_string_list (kf, "main", "plugins", NULL, NULL);
> +			priv->plugins = g_key_file_get_string_list (kf, "main", "plugins", NULL, NULL);
>  
> -		config->dhcp_client = g_key_file_get_value (kf, "main", "dhcp", NULL);
> -		config->dns_plugins = g_key_file_get_string_list (kf, "main", "dns", NULL, NULL);
> +		priv->dhcp_client = g_key_file_get_value (kf, "main", "dhcp", NULL);
> +		priv->dns_plugins = g_key_file_get_string_list (kf, "main", "dns", NULL, NULL);
>  
>  		if (cli_log_level && strlen (cli_log_level))
> -			config->log_level = g_strdup (cli_log_level);
> +			priv->log_level = g_strdup (cli_log_level);
>  		else
> -			config->log_level = g_key_file_get_value (kf, "logging", "level", NULL);
> +			priv->log_level = g_key_file_get_value (kf, "logging", "level", NULL);
>  
>  		if (cli_log_domains && strlen (cli_log_domains))
> -			config->log_domains = g_strdup (cli_log_domains);
> +			priv->log_domains = g_strdup (cli_log_domains);
>  		else
> -			config->log_domains = g_key_file_get_value (kf, "logging", "domains", NULL);
> +			priv->log_domains = g_key_file_get_value (kf, "logging", "domains", NULL);
>  
>  		if (cli_connectivity_uri && strlen (cli_connectivity_uri))
> -			config->connectivity_uri = g_strdup (cli_connectivity_uri);
> +			priv->connectivity_uri = g_strdup (cli_connectivity_uri);
>  		else
> -			config->connectivity_uri = g_key_file_get_value (kf, "connectivity", "uri", NULL);
> +			priv->connectivity_uri = g_key_file_get_value (kf, "connectivity", "uri", NULL);
>  
>  		if (cli_connectivity_interval >= 0)
> -			config->connectivity_interval = cli_connectivity_interval;
> +			priv->connectivity_interval = cli_connectivity_interval;
>  		else
> -			config->connectivity_interval = g_key_file_get_integer (kf, "connectivity", "interval", NULL);
> +			priv->connectivity_interval = g_key_file_get_integer (kf, "connectivity", "interval", NULL);
>  
>  		if (cli_connectivity_response && strlen (cli_connectivity_response))
> -			config->connectivity_response = g_strdup (cli_connectivity_response);
> +			priv->connectivity_response = g_strdup (cli_connectivity_response);
>  		else
> -			config->connectivity_response = g_key_file_get_value (kf, "connectivity", "response", NULL);
> +			priv->connectivity_response = g_key_file_get_value (kf, "connectivity", "response", NULL);
>  
>  		success = TRUE;
>  	}
> @@ -198,30 +289,33 @@ fill_from_file (NMConfig *config,
>  	return success;
>  }
>  
> +/* call this function only once! */
>  NMConfig *
>  nm_config_new (const char *cli_config_path,
> -               const char *cli_plugins,
> -               const char *cli_log_level,
> -               const char *cli_log_domains,
> -               const char *cli_connectivity_uri,
> -               const gint cli_connectivity_interval,
> -               const char *cli_connectivity_response,
> -               GError **error)
> +			   const char *cli_plugins,
> +			   const char *cli_log_level,
> +			   const char *cli_log_domains,
> +			   const char *cli_connectivity_uri,
> +			   const gint cli_connectivity_interval,
> +			   const char *cli_connectivity_response,
> +			   GError **error)
>  {
> -	NMConfig *config;
>  	GError *local = NULL;
> +	NMConfigPrivate *priv = NULL;
>  
> -	config = g_malloc0 (sizeof (*config));
> +	g_assert (!singleton);
> +	singleton = NM_CONFIG (g_object_new (NM_TYPE_CONFIG, NULL));
> +	priv = NM_CONFIG_GET_PRIVATE (singleton);
>  
>  	if (cli_config_path) {
>  		/* Bad user-specific config file path is a hard error */
> -		if (!fill_from_file (config, cli_config_path, cli_plugins, cli_log_level, cli_log_domains,
> +		if (!fill_from_file (singleton, cli_config_path, cli_plugins, cli_log_level, cli_log_domains,
>  		                     cli_connectivity_uri, cli_connectivity_interval, cli_connectivity_response,
>  		                     error)) {
> -			nm_config_free (config);
> -			return NULL;
> +			g_object_unref (singleton);
> +			singleton = NULL;
>  		}
> -		return config;
> +		return singleton;
>  	}
>  
>  	/* Even though we prefer NetworkManager.conf, we need to check the
> @@ -232,10 +326,10 @@ nm_config_new (const char *cli_config_path,
>  	 */
>  
>  	/* Try deprecated nm-system-settings.conf first */
> -	if (fill_from_file (config, NM_OLD_SYSTEM_CONF_FILE, cli_plugins, cli_log_level, cli_log_domains,
> +	if (fill_from_file (singleton, NM_OLD_SYSTEM_CONF_FILE, cli_plugins, cli_log_level, cli_log_domains,
>  	                    cli_connectivity_uri, cli_connectivity_interval, cli_connectivity_response,
>  	                    &local))
> -		return config;
> +		return singleton;
>  
>  	if (g_error_matches (local, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_NOT_FOUND) == FALSE) {
>  		fprintf (stderr, "Default config file %s invalid: (%d) %s\n",
> @@ -246,10 +340,10 @@ nm_config_new (const char *cli_config_path,
>  	g_clear_error (&local);
>  
>  	/* Try the standard config file location next */
> -	if (fill_from_file (config, NM_DEFAULT_SYSTEM_CONF_FILE, cli_plugins, cli_log_level, cli_log_domains,
> +	if (fill_from_file (singleton, NM_DEFAULT_SYSTEM_CONF_FILE, cli_plugins, cli_log_level, cli_log_domains,
>  	                    cli_connectivity_uri, cli_connectivity_interval, cli_connectivity_response,
>  	                    &local))
> -		return config;
> +		return singleton;
>  
>  	if (g_error_matches (local, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_NOT_FOUND) == FALSE) {
>  		fprintf (stderr, "Default config file %s invalid: (%d) %s\n",
> @@ -257,38 +351,21 @@ nm_config_new (const char *cli_config_path,
>  		         local ? local->code : -1,
>  		         (local && local->message) ? local->message : "unknown");
>  		g_propagate_error (error, local);
> -		nm_config_free (config);
> -		return NULL;
> +		g_object_unref (singleton);
> +		singleton = NULL;
> +		return singleton;
>  	}
>  
>  	/* If for some reason no config file exists, and NM wasn't given on on
>  	 * the command line, just use the default config file path.
>  	 */
> -	if (config->path == NULL) {
> -		config->path = g_strdup (NM_DEFAULT_SYSTEM_CONF_FILE);
> +	if (priv->path == NULL) {
> +		priv->path = g_strdup (NM_DEFAULT_SYSTEM_CONF_FILE);
>  		fprintf (stderr, "No config file found or given; using %s\n",
>  		         NM_DEFAULT_SYSTEM_CONF_FILE);
>  	}
>  
>  	/* ignore error if config file not found */
>  	g_clear_error (&local);
> -	return config;
> +	return singleton;
>  }
> -
> -void
> -nm_config_free (NMConfig *config)
> -{
> -	g_return_if_fail (config != NULL);
> -
> -	g_free (config->path);
> -	g_strfreev (config->plugins);
> -	g_free (config->dhcp_client);
> -	g_strfreev (config->dns_plugins);
> -	g_free (config->log_level);
> -	g_free (config->log_domains);
> -	g_free (config->connectivity_uri);
> -	g_free (config->connectivity_response);
> -	memset (config, 0, sizeof (*config));
> -	g_free (config);
> -}
> -
> diff --git a/src/nm-config.h b/src/nm-config.h
> index 6c4206c..ad8e8ef 100644
> --- a/src/nm-config.h
> +++ b/src/nm-config.h
> @@ -16,6 +16,7 @@
>   * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
>   *
>   * Copyright (C) 2011 Red Hat, Inc.
> + * Copyright (C) 2013 Thomas Bechtold <thomasbechtold jpberlin de>
>   */
>  
>  #ifndef NM_CONFIG_H
> @@ -24,7 +25,24 @@
>  #include <glib.h>
>  #include <glib-object.h>
>  
> -typedef struct NMConfig NMConfig;
> +G_BEGIN_DECLS
> +
> +#define NM_TYPE_CONFIG			  (nm_config_get_type ())
> +#define NM_CONFIG(obj)			  (G_TYPE_CHECK_INSTANCE_CAST ((obj), NM_TYPE_CONFIG, NMConfig))
> +#define NM_CONFIG_CLASS(klass)	  (G_TYPE_CHECK_CLASS_CAST ((klass),  NM_TYPE_CONFIG, NMConfigClass))
> +#define NM_IS_CONFIG(obj)         (G_TYPE_CHECK_INSTANCE_TYPE ((obj), NM_TYPE_CONFIG))
> +#define NM_IS_CONFIG_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass),  NM_TYPE_CONFIG))
> +#define NM_CONFIG_GET_CLASS(obj)  (G_TYPE_INSTANCE_GET_CLASS ((obj),  NM_TYPE_CONFIG, NMConfigClass))
> +
> +typedef struct {
> +	GObject parent;
> +} NMConfig;
> +
> +typedef struct {
> +	GObjectClass parent;
> +} NMConfigClass;
> +
> +GType nm_config_get_type (void);
>  
>  typedef enum {
>  	NM_CONFIG_ERROR_NO_MEMORY = 0, /*< nick=NoMemory >*/
> @@ -33,15 +51,15 @@ typedef enum {
>  #define NM_CONFIG_ERROR (nm_config_error_quark ())
>  GQuark nm_config_error_quark (void);
>  
> -
> +NMConfig *nm_config_get (void);
>  NMConfig *nm_config_new (const char *cli_config_path,
> -                         const char *cli_plugins,
> -                         const char *cli_log_level,
> -                         const char *cli_log_domains,
> -                         const char *cli_connectivity_check_uri,
> -                         const gint connectivity_check_interval,
> -                         const char *cli_connectivity_check_response,
> -                         GError **error);
> +						 const char *cli_plugins,
> +						 const char *cli_log_level,
> +						 const char *cli_log_domains,
> +						 const char *cli_connectivity_check_uri,
> +						 const gint connectivity_check_interval,
> +						 const char *cli_connectivity_check_response,
> +						 GError **error);
>  
>  const char *nm_config_get_path (NMConfig *config);
>  const char **nm_config_get_plugins (NMConfig *config);
> @@ -53,7 +71,7 @@ const char *nm_config_get_connectivity_uri (NMConfig *config);
>  const guint nm_config_get_connectivity_interval (NMConfig *config);
>  const char *nm_config_get_connectivity_response (NMConfig *config);
>  
> -void nm_config_free (NMConfig *config);
> +G_END_DECLS
>  
>  #endif /* NM_CONFIG_H */
>
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]