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. Re: [Patch] Bug fix when switching happens from Auto to
Manual (Daniel Wagner)
2. Re: [PATCH] service: do not reply twice to Connect (Daniel Wagner)
3. Re: [PATCH] vpn: Fix memory leak (Daniel Wagner)
4. Re: [PATCH] vpn: Removed unnecessary goto (Daniel Wagner)
5. Re: [PATCH] inet: Invoke rtnl callback on errors too
(Daniel Wagner)
6. Re: [PATCH] fixed noisy error Cannot read /proc/net/pnp
Failed to open file (Daniel Wagner)
----------------------------------------------------------------------
Message: 1
Date: Wed, 22 Aug 2018 08:03:44 +0200
From: Daniel Wagner <[email protected]>
To: [email protected]
Cc: "[email protected]" <[email protected]>, AMIT KUMAR JAISWAL
<[email protected]>
Subject: Re: [Patch] Bug fix when switching happens from Auto to
Manual
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Rahul,
This emails is again white space damaged and this time even send as
HTML. There are excellent guide how to send proper patches even using
broken corporate emails servers. Since I already invested time reading
the patch I opted for fixing it up myself. But please fix your email
setup for next time.
The patch title is not following our code guidelines. You should prefix
it. In this case "network:". Also calling it 'bug fix' is a bit of
noise. Just state what you do in the patch. In the commit message you
should explain why you do the change.
On 08/17/2018 06:43 AM, Rahul Jain wrote:
> ??? When a device having auto IP configurations, is connected
> ??? to router in which DHCP is not running, device will go for
> ??? IPv4All configuration and will get IP(like 169.254.xxx.xxx).
> ??? Inside connman/src/dhcp.c:dhcpipv4ll_available_cb(), dhcp ipconfig
> set to AUTO.
> ??? Now if user tries to configure manual IP, it fails.
> ??? As per current code, dhcp stop function (__connman_dhcp_stop) will
> not be called for IPv5.
I suppose this should be IPv4.
> diff --git a/src/network.c b/src/network.c
> index c3a7cbf..2d1cbb9 100644
> --- a/src/network.c
> +++ b/src/network.c
> @@ -1872,12 +1872,13 @@ int __connman_network_clear_ipconfig(struct
> connman_network *network,
> ??case CONNMAN_IPCONFIG_METHOD_OFF:
> ??case CONNMAN_IPCONFIG_METHOD_FIXED:
> ???return -EINVAL;
> -?case CONNMAN_IPCONFIG_METHOD_AUTO:
> -??release_dhcpv6(network);
> -??break;
> ??case CONNMAN_IPCONFIG_METHOD_MANUAL:
> ???__connman_ipconfig_address_remove(ipconfig);
> ???break;
> +?case CONNMAN_IPCONFIG_METHOD_AUTO:
> +??release_dhcpv6(network);
> +??if (type == CONNMAN_IPCONFIG_TYPE_IPV6)
> +???break;
src/network.c: In function ?__connman_network_clear_ipconfig?:
src/network.c:1880:6: error: this statement may fall through
[-Werror=implicit-fallthrough=]
if (type == CONNMAN_IPCONFIG_TYPE_IPV6)
^
src/network.c:1882:2: note: here
case CONNMAN_IPCONFIG_METHOD_DHCP:
^~~~
cc1: all warnings being treated as errors
I fixed this by adding a "/* fall through */" comment. GCC is then
happy with the code.
Patch applied.
Thanks,
Daniel
------------------------------
Message: 2
Date: Wed, 22 Aug 2018 08:27:07 +0200
From: Daniel Wagner <[email protected]>
To: Beno?t Monin <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH] service: do not reply twice to Connect
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Beno?t,
Sorry for the delay.
>> I am not convinced that your patch is complete correct. In case where
>> __connman_service_connect() returns with success instead of EINPROGRESS
>> then your patch could it up the response, no?
>>
>> Suppose that autoconnect is happening in the background and the user
>> calls 'Connect' before the Service was online. In this case we used to
>> send an 'error already' back.
>>
>> The question is why does ConnMan send the same response twice? Can you
>> send a log?
>>
> I assumed that in function connect_service, upon return from
> __connman_service_connect if service->pending is NULL, it means that a
> reply has been sent. Depending on the code path taken from
> __connman_service_connect, if there is a call to reply_pending, then the
> first reply is sent and service->pending is sets to NULL.
Ah! I didn't see the reply_pending() inside __connman_service_connect().
That explains the double send.
> Maybe I should make the patch more explicit, something like:
> if service->pending is null then return null? Or do you think there are
> places where service->pending can be set to NULL without sending a reply?
I think the problem is that we do have two different expectation on how
__connman_service_connect() should handle errors. In most cases
__connman_service_connect() just returns the error code. Currently only
service_connect() is checking the return value and sends then a reply in
the error case. Also __connman_service_disconnect() has this two kinds
of error handling.
Hmm, what about moving reply_pending() out of
__connman_service_connect() and __connman_service_disconnect() and send
the error message from the calling side? This would make it more
explicit what part of the code is in charge for sending messages. Note
we currently write 'reply_pending(service)' which already indicates
something is wrong
Thanks,
Daniel
------------------------------
Message: 3
Date: Wed, 22 Aug 2018 08:35:04 +0200
From: Daniel Wagner <[email protected]>
To: Slava Monich <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH] vpn: Fix memory leak
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Slava,
On 08/14/2018 10:16 PM, Slava Monich wrote:
> ==10939== 20 bytes in 4 blocks are definitely lost in loss record 199 of 429
> ==10939== at 0x483F3EC: malloc (vg_replace_malloc.c)
> ==10939== by 0x4C7E35F: g_malloc (gmem.c)
> ==10939== by 0x4C962BD: g_strdup (gstrfuncs.c)
> ==10939== by 0x1945B: vpn_create_tun (vpn.c)
> ==10939== by 0x19C83: vpn_connect (vpn.c)
> ==10939== by 0x292DF: __vpn_provider_connect (vpn-provider.c)
> ==10939== by 0x27797: do_connect (vpn-provider.c)
> ==10939== by 0x46853: process_message (object.c)
> ==10939== by 0x486AB: generic_message (object.c)
> ==10939== by 0x49BDFE1: _dbus_object_tree_dispatch_and_unlock
> (dbus-object-tree.c)
> ==10939== by 0x49B4071: dbus_connection_dispatch (dbus-connection.c)
> ==10939== by 0x43A8B: message_dispatch (mainloop.c)
> ---
Patch applied (changed the commit title slightly)
Thanks,
Daniel
------------------------------
Message: 4
Date: Wed, 22 Aug 2018 08:37:16 +0200
From: Daniel Wagner <[email protected]>
To: Slava Monich <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH] vpn: Removed unnecessary goto
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Slava,
Patch applied!
Thanks,
Daniel
------------------------------
Message: 5
Date: Wed, 22 Aug 2018 08:38:52 +0200
From: Daniel Wagner <[email protected]>
To: Slava Monich <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH] inet: Invoke rtnl callback on errors too
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Slava,
Patch applied!
Thanks,
Daniel
------------------------------
Message: 6
Date: Wed, 22 Aug 2018 08:52:46 +0200
From: Daniel Wagner <[email protected]>
To: Vasyl Vavrychuk <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH] fixed noisy error Cannot read /proc/net/pnp
Failed to open file
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Vasyl,
On 08/21/2018 10:23 AM, Vasyl Vavrychuk wrote:
> Hello Daniel,
>
> This patch still needs to be applied because I am getting
>
> __connman_inet_get_pnp_nameservers: Cannot read /proc/net/pnp Failed to
> open file '/proc/net/pnp': No such file or directory
Argh, sorry about that. IIRC I was pointing out that I don't get why it
is needed but now I realize what the problem is. We have two places
where we want to open '/proc/net/pnp'. get_nfs_server_ip() and
__connman_inet_get_pnp_nameservers() and I was starring at the first
function which has the check... Obviously, the second one needs this
treatment as well.
Patch applied. Sorry for the inconvenience.
Thanks for reminding me!
Daniel
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 34, Issue 11
***************************************
