Re: [PATCH v2 1/5] A new object 'NMProxyConfig' with fields for proxy.

[Atul Anand <atulhjp gmail com>]

On 7/27/16, Beniamino Galvani <bgalvani redhat com> wrote:
> On Tue, Jul 12, 2016 at 05:57:16PM +0530, Atul Anand wrote:
> > A new object NMProxyConfig has been implemented which contain fields
> > for proxy related stuff.
>
> Could you please prefix each commit subject with the component or area
> affected by the change as:
>
>   core: ...
>
> or
>
>   proxy: ...
>
> Would it be possible to change the order of commits so that the
> libnm-core patch comes before (as this patch depends on it)?
>
> > +void
> > +nm_proxy_config_merge_setting (NMProxyConfig *config, NMSettingProxy
> > *setting)
> > +{
> > [...]
> > +    priv->proxies = NULL;
> > +    priv->excludes = NULL;
> > +    priv->pac_url = NULL;
> > +    priv->pac_script = NULL;
>
> It seems this clears all existing parameters, including the pac_url
> previously set (for example from DHCP), is it correct?
>
> Beniamino
I haven't tested this as i haven't a DHCP server with configured
pac_url. But nm_proxy_config_merge_setting() is invoked in
dhcp4_config_changed(). So, Shouldn't we override the config
parameters with NMSettingProxy if the later one has been previously
set by user or by nm_proxy_config_create_setting() when connection
starts up.