On Tue, Aug 02, 2016 at 02:01:05AM +0530, Atul Anand wrote:
libnm-core has been expanded to include proxy settings which clients like nmcli, nm-connection-editor use to configure proxy in PacRunner. It offers three modes i.e 'auto', 'manual'and 'none' and accordingly take data to configure PacRunner. The modes matches on the PacRunner side too.
Hi, the series looks good in general. Some comments below.
--- a/libnm-core/nm-connection.c +++ b/libnm-core/nm-connection.c @@ -2137,6 +2137,22 @@ nm_connection_get_setting_pppoe (NMConnection *connection) } /** + * nm_connection_get_setting_proxy: + * @connection: the #NMConnection + * + * A shortcut to return any #NMSettingProxy the connection might contain. + * + * Returns:an #NMSettingProxy if the connection contains one, otherwise %NULL
Nitpick: this is public API and needs the "Since" tag...
--- a/libnm-core/nm-connection.h +++ b/libnm-core/nm-connection.h @@ -211,6 +211,7 @@ NMSettingMacvlan * nm_connection_get_setting_macvlan (NMConnec NMSettingOlpcMesh * nm_connection_get_setting_olpc_mesh (NMConnection *connection); NMSettingPpp * nm_connection_get_setting_ppp (NMConnection *connection); NMSettingPppoe * nm_connection_get_setting_pppoe (NMConnection *connection); +NMSettingProxy * nm_connection_get_setting_proxy (NMConnection *connection);
... and NM_AVAILABLE_IN_1_2 here.
+ if (method == NM_SETTING_PROXY_METHOD_NONE) {
+ if (priv->pac_url) {
+ g_set_error (error,
+ NM_CONNECTION_ERROR,
+ NM_CONNECTION_ERROR_INVALID_PROPERTY,
+ _("this property is not allowed for method=manual/none"));
"for method none"
+
+ if (priv->pac_script) {
+ g_set_error (error,
+ NM_CONNECTION_ERROR,
+ NM_CONNECTION_ERROR_INVALID_PROPERTY,
+ _("this property is not allowed for method=manual/none"));
Ditto.
+ } else if (method == NM_SETTING_PROXY_METHOD_MANUAL) {
+ if (priv->pac_url) {
+ g_set_error (error,
+ NM_CONNECTION_ERROR,
+ NM_CONNECTION_ERROR_INVALID_PROPERTY,
+ _("this property is not allowed for method=manual/none"));
"for method manual".
+ if (priv->pac_script) {
+ g_set_error (error,
+ NM_CONNECTION_ERROR,
+ NM_CONNECTION_ERROR_INVALID_PROPERTY,
+ _("this property is not allowed for method=manual/none"));
Ditto
+ g_prefix_error (error, "%s.%s: ", NM_SETTING_PROXY_SETTING_NAME, NM_SETTING_PROXY_PAC_SCRIPT); + return FALSE; + } + } else + return FALSE;
@error must be always set when returning FALSE.
+static void +set_property (GObject *object, guint prop_id, + const GValue *value, GParamSpec *pspec) + [...] + case PROP_SSL_PROXY: + g_free (priv->ssl_proxy); + /* Check if HTTP Proxy has been set for all Protocols */ + priv->ssl_proxy = priv->http_default == TRUE ? g_strdup (priv->http_proxy) : g_value_dup_string (value);
In my opinion here we should simply assign @value to priv->ssl_proxy; the fallback to default should be something done by the caller. But if we really must do it here, @http_default is already a gboolean and so the "== TRUE" is not needed. Beniamino
Attachment:
signature.asc
Description: PGP signature