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] supplicant: fixed return value of ssid getter
(Daniel Wagner)
2. Re: [PATCH] fixed noisy warning on every connect (Daniel Wagner)
3. Re: [PATCH v2] build: Add --disable-stats option (Daniel Wagner)
----------------------------------------------------------------------
Message: 1
Date: Wed, 28 Feb 2018 08:37:44 +0100
From: Daniel Wagner <[email protected]>
To: Vasyl Vavrychuk <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH] supplicant: fixed return value of ssid getter
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Good Morning Vasyl,
On 02/24/2018 05:29 PM, Vasyl Vavrychuk wrote:
> By previously I mean before my commit.
>
> Without my commit for hidden networks g_supplicant_network_get_ssid
> returns ssid_len=0 and as a return value the pointer to the empty
> string.
>
> In network_added this return value is assigned to variable ssid
>
> ssid = g_supplicant_network_get_ssid(supplicant_network, &ssid_len);
>
> and later in network_added this pointer is checked for not NULL
>
> if (ssid)
> connman_network_set_group(network, group);
>
> if (wifi->hidden && ssid) {
> ...
> }
>
> what is the point of checking for not NULL if it is always not NULL?
from gsupplicant/supplicant.c"
const void *g_supplicant_network_get_ssid(GSupplicantNetwork *network,
unsigned int *ssid_len)
{
if (!network) {
*ssid_len = 0;
return NULL;
}
*ssid_len = network->ssid_len;
return network->ssid;
}
static int add_or_replace_bss_to_network(struct g_supplicant_bss *bss)
{
[...]
network->ssid_len = bss->ssid_len;
memcpy(network->ssid, bss->ssid, bss->ssid_len);
[...]
}
If we hand it an empty network ssid will be NULL. No surprise there. The
other case I see is that we get an empty ssid from wpa_s.
So when we check for 'network->ssid_len == 0' in
g_supplicant_network_get_ssid, makes a difference. In this case we would
also skip the network creation from your code snippet above. Or do I
miss something.
Now I got the impression that this part here is not really consistently
coded. Can't we say if 'ssid_len == 0', then must be ssid == NULL?
Therefore we check for ssid_len == 0 instead of ssdi == NULL where this
matters?
> Another thing is that connman_network_set_blob(network, "WiFi.SSID",
> ssid="", ssid_len=0); will set network->wifi.ssid=NULL.
Ah, so that's the difference. 'ssid_len == 0' means also emtpy string.
That explains my question from above. Argh!
What about introducing a helper function which says, the SSID we have is
a valig, something like this
bool ssid_is_valid(const unsigned char *ssid, unsigned int ssid_len)
{
/* empty ssid, that is "" */
if (ssid_len == 0 && !ssid && ssid[0] == '\0')
return true;
if (ssid_len > 0)
return true;
return false;
}
and use it everyhwere we need. Open coding seems like a bad idea.
Thanks,
Daniel
------------------------------
Message: 2
Date: Wed, 28 Feb 2018 08:45:16 +0100
From: Daniel Wagner <[email protected]>
To: Vasyl Vavrychuk <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH] fixed noisy warning on every connect
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Vasyl,
On 02/24/2018 05:10 PM, Vasyl Vavrychuk wrote:
>>> On every connect I get 'Skipping disconnect of ..., network is
>>> connecting'.
>>> This is happening because __connman_network_connect sets connecting
>>> flag before __connman_device_disconnect. The last function then prints
>>> warning due to this connecting flag.
>>>
>>> Changed order of assigning connecting and __connman_device_disconnect
>>> fixes this. Connman logic is not effected due to the way how flags
>>> connected and associating are handled in __connman_network_connect
>>> and __connman_network_disconnect.
>>> ---
>>> src/network.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/network.c b/src/network.c
>>> index 8c5979f7..4757d0d7 100644
>>> --- a/src/network.c
>>> +++ b/src/network.c
>>> @@ -1468,10 +1468,10 @@ int __connman_network_connect(struct
>>> connman_network *network)
>>> if (!network->device)
>>> return -ENODEV;
>>> - network->connecting = true;
>>> -
>>> __connman_device_disconnect(network->device);
>>> + network->connecting = true;
>>> +
>>> err = network->driver->connect(network);
>>> if (err < 0) {
>>> if (err == -EINPROGRESS)
>>>
>>
>> I found this part of always confusing but never took time to really try to
>> figure out why it is there. After some digging in the history I think we can
>> safely remove the __connman_device_disconnect() call instead of moving the
>> network->connecting part. With setting network->connecting = true we have a
>> no-op in __connman_device_disconnect(). So why bother to call it anyway from
>> here?
>
> Suppose we are connecting to network A belonging to device B. But
> besides network A
> there can be other networks belonging to device B. And
> __connman_device_disconnect
> will perform disconnect from those networks. How about this? Can we
> still safely remove
> __connman_device_disconnect?
Yes, you are right. I'll apply this patch then. Just a sec.
Thanks,
Daniel
------------------------------
Message: 3
Date: Wed, 28 Feb 2018 09:07:08 +0100
From: Daniel Wagner <[email protected]>
To: Chris Novakovic <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH v2] build: Add --disable-stats option
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Chris,
On 02/26/2018 10:38 PM, Chris Novakovic wrote:
> Generation of interface statistics files can now be controlled at
> compile-time using the --{enable,disable}-stats configure options.
> Statistics files remain enabled by default.
>
> Based on an idea by Feng Wang <[email protected]>.
Thanks for updating the commit message
Though these in-file ifdefs are not okay. Create a new file, e.g.
nostats.c, which contains the empty implementation of the functions. And
then include according the configuration either the stats.c or nostats.c
into the build.
See how this has been done for WISPR.
if WISPR
gweb_sources += gweb/giognutls.h gweb/giognutls.c
else
gweb_sources += gweb/giognutls.h gweb/gionotls.c
endif
Thanks,
Daniel
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 28, Issue 23
***************************************
