Send connman mailing list submissions to
[email protected]
To subscribe or unsubscribe via the World Wide Web, visit
https://lists.01.org/mailman/listinfo/connman
or, via email, send a message with subject or body 'help' to
[email protected]
You can reach the person managing the list at
[email protected]
When replying, please edit your Subject line so it is more specific
than "Re: Contents of connman digest..."
Today's Topics:
1. [PATCH 1/2] proxy: Fix handling of proxy->domains
(David Woodhouse)
2. Re: [PATCH 1/2] proxy: Fix handling of proxy->domains
(Tomasz Bursztyka)
3. Re: [PATCH 1/2] proxy: Fix handling of proxy->domains
(David Woodhouse)
4. [PATCH v2 1/2] proxy: Fix handling of proxy->domains
(David Woodhouse)
----------------------------------------------------------------------
Message: 1
Date: Fri, 17 Jun 2016 11:35:42 +0100
From: David Woodhouse <[email protected]>
To: [email protected]
Subject: [PATCH 1/2] proxy: Fix handling of proxy->domains
Message-ID: <[email protected]>
Content-Type: text/plain; charset="utf-8"
In create_proxy_config() we were calling pacrunner_proxy_set_domains(),
which was returning an error if passed a NULL list of domains.
Then we were calling pacrunner_proxy_set_auto() or one of its friends,
which all call reset_proxy() and *delete* the list of domains, if one
was set.
Treat proxy->domains like proxy->interface, and don't reset it in
reset_proxy(). And fix up pacrunner_proxy_set_domains() so that it
happily accepts a NULL domains list and behaves appropriately. As
a side-effect, this stops pacrunner_proxy_set_domains() from being
additive ? if you call it a second time it'll clear the original set
and set just the ones you've just passed, instead of adding new
domains to the list. Which is a much saner semantic.
---
src/proxy.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/src/proxy.c b/src/proxy.c
index 2ebc75d..70efd98 100644
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -115,10 +115,6 @@ static void reset_proxy(struct pacrunner_proxy *proxy)
__pacrunner_manual_destroy_excludes(proxy->excludes);
proxy->excludes = NULL;
-
- if (proxy->domains)
- g_list_free_full(proxy->domains, proxy_domain_destroy);
- proxy->domains = NULL;
}
void pacrunner_proxy_unref(struct pacrunner_proxy *proxy)
@@ -133,6 +129,10 @@ void pacrunner_proxy_unref(struct pacrunner_proxy *proxy)
reset_proxy(proxy);
+ if (proxy->domains)
+ g_list_free_full(proxy->domains, proxy_domain_destroy);
+ proxy->domains = NULL;
+
g_free(proxy->interface);
g_free(proxy);
}
@@ -168,8 +168,13 @@ int pacrunner_proxy_set_domains(struct pacrunner_proxy
*proxy, char **domains)
if (!proxy)
return -EINVAL;
+ if (proxy->domains)
+ g_list_free_full(proxy->domains, proxy_domain_destroy);
+
+ proxy->domains = NULL;
+
if (!domains)
- return -EINVAL;
+ return 0;
for (domain = domains; *domain; domain++) {
struct proxy_domain *data;
--
2.7.4
--
David Woodhouse Open Source Technology Centre
[email protected] Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 5760 bytes
Desc: not available
URL:
<http://lists.01.org/pipermail/connman/attachments/20160617/e9d88c0f/attachment-0001.bin>
------------------------------
Message: 2
Date: Fri, 17 Jun 2016 12:59:25 +0200
From: Tomasz Bursztyka <[email protected]>
To: [email protected]
Subject: Re: [PATCH 1/2] proxy: Fix handling of proxy->domains
Message-ID: <[email protected]>
Content-Type: text/plain; charset=windows-1252; format=flowed
Hi David,
> In create_proxy_config() we were calling pacrunner_proxy_set_domains(),
> which was returning an error if passed a NULL list of domains.
>
> Then we were calling pacrunner_proxy_set_auto() or one of its friends,
> which all call reset_proxy() and *delete* the list of domains, if one
> was set.
>
> Treat proxy->domains like proxy->interface, and don't reset it in
> reset_proxy(). And fix up pacrunner_proxy_set_domains() so that it
> happily accepts a NULL domains list and behaves appropriately. As
> a side-effect, this stops pacrunner_proxy_set_domains() from being
> additive ? if you call it a second time it'll clear the original set
> and set just the ones you've just passed, instead of adding new
> domains to the list. Which is a much saner semantic.
> ---
> src/proxy.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/src/proxy.c b/src/proxy.c
> index 2ebc75d..70efd98 100644
> --- a/src/proxy.c
> +++ b/src/proxy.c
> @@ -115,10 +115,6 @@ static void reset_proxy(struct pacrunner_proxy *proxy)
>
> __pacrunner_manual_destroy_excludes(proxy->excludes);
> proxy->excludes = NULL;
> -
> - if (proxy->domains)
> - g_list_free_full(proxy->domains, proxy_domain_destroy);
> - proxy->domains = NULL;
> }
>
> void pacrunner_proxy_unref(struct pacrunner_proxy *proxy)
> @@ -133,6 +129,10 @@ void pacrunner_proxy_unref(struct pacrunner_proxy *proxy)
>
> reset_proxy(proxy);
>
> + if (proxy->domains)
> + g_list_free_full(proxy->domains, proxy_domain_destroy);
Add a new line here (yep, I missed that one in original code)
> + proxy->domains = NULL;
> +
> g_free(proxy->interface);
> g_free(proxy);
> }
> @@ -168,8 +168,13 @@ int pacrunner_proxy_set_domains(struct pacrunner_proxy
> *proxy, char **domains)
> if (!proxy)
> return -EINVAL;
>
> + if (proxy->domains)
> + g_list_free_full(proxy->domains, proxy_domain_destroy);
> +
> + proxy->domains = NULL;
> +
> if (!domains)
> - return -EINVAL;
> + return 0;
>
Rest is fine to me.
Tomasz
------------------------------
Message: 3
Date: Fri, 17 Jun 2016 12:28:03 +0100
From: David Woodhouse <[email protected]>
To: Tomasz Bursztyka <[email protected]>,
[email protected]
Subject: Re: [PATCH 1/2] proxy: Fix handling of proxy->domains
Message-ID: <[email protected]>
Content-Type: text/plain; charset="utf-8"
On Fri, 2016-06-17 at 12:59 +0200, Tomasz Bursztyka wrote:
> > @@ -133,6 +129,10 @@ void pacrunner_proxy_unref(struct pacrunner_proxy
> > *proxy)
> >??
> >???????reset_proxy(proxy);
> >??
> > +?????if (proxy->domains)
> > +?????????????g_list_free_full(proxy->domains, proxy_domain_destroy);
>
> Add a new line here (yep, I missed that one in original code)
No, it's the other way round actually. Instead of adding a blank line here...
> > +?????proxy->domains = NULL;
> > +
> >???????g_free(proxy->interface);
> >???????g_free(proxy);
> >?? }
> > @@ -168,8 +168,13 @@ int pacrunner_proxy_set_domains(struct pacrunner_proxy
> > *proxy, char **domains)
> >???????if (!proxy)
> >???????????????return -EINVAL;
> >??
> > +?????if (proxy->domains)
> > +?????????????g_list_free_full(proxy->domains, proxy_domain_destroy);
> > +
... we should be *removing* this one.
> > +?????proxy->domains = NULL;
> > +
> >???????if (!domains)
> > -?????????????return -EINVAL;
> > +?????????????return 0;
> >??
Because logically, it's one operation, to free-and-clear a pointer.
If we could write it as
proxy->domains = g_list_free_full(?);
we would.
Look at all the rest of reset_proxy(), where we do it precisely the
same. Free and clear in pairs, then a blank line before the next free-
and-clear pair:
g_free(proxy->url);
proxy->url = NULL;
g_free(proxy->script);
proxy->script = NULL;
__pacrunner_manual_destroy_servers(proxy->servers);
proxy->servers = NULL;
__pacrunner_manual_destroy_excludes(proxy->excludes);
proxy->excludes = NULL;
It was right the first time. It was right when Atul added it to
pacrunner_proxy_unref(), and I only got it *wrong* when I extended his
patch and also added the same to pacrunner_proxy_set_domains()
I'll submit a new patch.
--
dwmw2
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 5760 bytes
Desc: not available
URL:
<http://lists.01.org/pipermail/connman/attachments/20160617/4895ed7c/attachment-0001.bin>
------------------------------
Message: 4
Date: Fri, 17 Jun 2016 12:33:53 +0100
From: David Woodhouse <[email protected]>
To: [email protected]
Subject: [PATCH v2 1/2] proxy: Fix handling of proxy->domains
Message-ID: <[email protected]>
Content-Type: text/plain; charset="utf-8"
In create_proxy_config() we were calling pacrunner_proxy_set_domains(),
which was returning an error if passed a NULL list of domains.
Then we were calling pacrunner_proxy_set_auto() or one of its friends,
which all call reset_proxy() and *delete* the list of domains, if one
was set.
Treat proxy->domains like proxy->interface, and don't reset it in
reset_proxy(). And fix up pacrunner_proxy_set_domains() so that it
happily accepts a NULL domains list and behaves appropriately. As
a side-effect, this stops pacrunner_proxy_set_domains() from being
additive ? if you call it a second time it'll clear the original set
and set just the ones you've just passed, instead of adding new
domains to the list. Which is a much saner semantic.
---
src/proxy.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/src/proxy.c b/src/proxy.c
index 2ebc75d..95ee75c 100644
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -115,10 +115,6 @@ static void reset_proxy(struct pacrunner_proxy *proxy)
__pacrunner_manual_destroy_excludes(proxy->excludes);
proxy->excludes = NULL;
-
- if (proxy->domains)
- g_list_free_full(proxy->domains, proxy_domain_destroy);
- proxy->domains = NULL;
}
void pacrunner_proxy_unref(struct pacrunner_proxy *proxy)
@@ -133,6 +129,10 @@ void pacrunner_proxy_unref(struct pacrunner_proxy *proxy)
reset_proxy(proxy);
+ if (proxy->domains)
+ g_list_free_full(proxy->domains, proxy_domain_destroy);
+ proxy->domains = NULL;
+
g_free(proxy->interface);
g_free(proxy);
}
@@ -168,8 +168,12 @@ int pacrunner_proxy_set_domains(struct pacrunner_proxy
*proxy, char **domains)
if (!proxy)
return -EINVAL;
+ if (proxy->domains)
+ g_list_free_full(proxy->domains, proxy_domain_destroy);
+ proxy->domains = NULL;
+
if (!domains)
- return -EINVAL;
+ return 0;
for (domain = domains; *domain; domain++) {
struct proxy_domain *data;
--
2.7.4
--
David Woodhouse Open Source Technology Centre
[email protected] Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 5760 bytes
Desc: not available
URL:
<http://lists.01.org/pipermail/connman/attachments/20160617/ab7d83dd/attachment.bin>
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 8, Issue 23
**************************************
