Re: [PATCH] doc: Add API documentation about WiFi P2P Peer

[Tomasz Bursztyka]

Hi,

As far as I know ConnMan use WPA_S without a such file, so I think 
ConnMan should be responsible for records and
managing those information, for example,  expose a interface to *get* 
those persistent group information, and then
a interfaces to*remove* it from ConnMan and WPA_S. 


There is no need to expose anything. Afaik, wpa_s exposes already useful 
information and methods for that through DBus API.

It might be buggy though, we will see.

Br,

Tomasz

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH] doc: Add API documentation about WiFi P2P Peer

[Tomasz Bursztyka]

Hi Martin,

Looks like we are on the same line

Indeed, ConnMan will handle necessary group stuff, but won't expose 
anything through its own DBus API.

Application don't need to know what's happening about that.


The goal is good, but way to let one peer to load configuration to create the 
GO manually is really stupid.
That should be done automatically and transparently to user.


Yep we should not let the application touching these.
However, in very specific cases, ConnMan might need to create the GO by 
itself (rather than asking for negotiating it).
For instance if you serve a display as a sink, you just want to be sure 
everything gets routed to you by default.

Of course, transparently as usual so applications won't notice anything.

Btw, I have started to work on service discovery/addition/deletion, it 
should come soon in the test script I did.
That will help us finding out how to expose services and methods related 
to those.


So handling this specific case will come when we will have decided what 
to expose about P2P services.



I think the connection configuration(SSID, GO, Credential, so called persistent 
group) should always be recorded.

So next time, one peer send connection requests,  and found that they ever be 
connected(Through MAC address), just load the configuration automatically.
And never need user involve. I think we can do that at WPA_S.


Exactly what I had in mind. As long as we detect the group we are part 
of as persistent one: we store the data (as we do for services etc...)

and handling a reconnection to it will be transparent.

Br,

Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH v1 4/6] session: Expand '*' to all bearer types

[Tomasz Bursztyka]

Hi Daniel,

I am fine with that patch, but after that: it might be a good idea to 
get rid of the
glist for allowed_bearers and switch it to a fixed sized array (size of 
default_bearers actually).


Actually, bearer2service() and service2bearer() could be improved as well:
get a static array of chars, of the size of default bearer, with the 
bearer names and operate on it directly.

Though that one is a minor issue.

Br,

Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH v1 5/6] session: Allow to set any string in AllowedBearers

[Tomasz Bursztyka]

Hi Daniel,

So if I read your patch properly, it means applications will be able to 
tell which service ID they want. Tell me if I am wrong


I understand the use case of filtering the network access by service 
type and/or service ID. And it is a corner-case in fact.

Letting apps to handle that if not the proper solution.

You have a nice policy mechanism, imho it's should be up to it to handle 
that and hide it from applications.


There is a actually many issues around that corner case, first big one 
is how to decide which service ID is allowed
since service ID is generated at run-time, you cannot provision that 
before hand.
(Note that your patch does not solve this as well from application side: 
how an app could hardcode such service ID etc...)


In any case, application should not mess up with that.

Br,

Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH] doc: Add API documentation about WiFi P2P Peer

[Tomasz Bursztyka]

Hi Martin and Patrik,

As I said in my cover letter: this first part only deals with 1:1 uses 
case, which is currently the only real ones in the fields, so no group 
at all here.
(and anyway: if there is a layer which should care about the group part, 
it's definitely not the application itself.)


I prefer to be conservative now, and exposing the less possible. It's 
far better to add useful things later, than scrapping useless ones 
afterwards since

people would have already started to use the API etc...


I know the API is compatible with ConnMan, since ConnMan does not
export device information, but I still think BlueZ's P2P API should be
referenced by us.

Good point. In due time, yes.


It's actually already following that path: p2p technology's Scan() is 
like a Discovery()
I just did not wanted to add this specific name, when Scan() exists 
already and when semantically it is very close: a scan discovers other 
entities.
And peer object are like device object. It just makes more sense to call 
it peer... as in peer-to-peer.


Rest will have to be evaluated like: some kind of profiles, etc...


+   Valid state are "idle", "failure", "association",
+   "configuration", "ready" and "disconnect".

"association" and "configuration" is useful?

This follows the service states, which is a plus. I'm not sure we'll
need all this state information, let's see.


Yep, I put all states as for services (minus "online" since obviously we 
are not supposed to get it).
I agree: whatever possible superfluous states should be removed, if 
necessary. No renaming though.



+   string Name [readonly]
+
+   Name of the peer.

We also need type, UI needs to show the device type.

When we manage to read through the spec up to this point, it will be
added.


If we have to export it, it should be through a proper string and 
nothing else.

We might stick with the primary value.
(Actually, a proper UI should show an icon related to this type, but 
that's not our part)


And still, a proper peer name should be sufficient here, so let's wait 
for that.

I am not keen on integrating it just yet.


Do we really need to export the peer IPV4 to user?
I think for Peer, MAC address is more useful

This would be our local IPv4 address. I think applications would like to
know that.


I am still questioning myself about exposing discovered peers MAC address.
In many P2P usage I have seen people caring more about it than the 
peer's name (aka: "device name")
Which is very stupid: a MAC is way less human readable than a proper 
name but anyway...



Br,

Tomasz

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH 3/3] connection: Remove the VPN's host routing

[Tomasz Bursztyka]

Hum, made a mistake too:



These outputs are the same, I guess I made a mistake in your copy/paste?


s/I/you

;)

Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH 3/3] connection: Remove the VPN's host routing

[Tomasz Bursztyka]

Hi,


>Oh, sorry, committed messages are wrong, they should be:

 Disabled VPN (Wrong):
 Kernel IP routing table
 Destination Gateway Genmask Flags   Iface
 0.0.0.0   192.168.1.1 0.0.0.0 U wlan0

 Disabled VPN (Correct):
 Kernel IP routing table
 Destination Gateway Genmask Flags   Iface
 0.0.0.0   192.168.1.1 0.0.0.0  Uwlan0

I will commit the second version later.


These outputs are the same, I guess I made a mistake in your copy/paste?

Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[RFC] WiFi P2P Peer API - v2

[Tomasz Bursztyka]

v2: Removed useless details or non relevant property (GroupName).

Second shot for the P2P API proposal.

It still focus on the simplest case first: Peer client, in a 1 to 1 context.

The idea is to create a new technology, whose hierarchical parent will be wifi 
technology.
(Thus if wifi is disabled, p2p is disabled. The other way round is not true)

I kept "Scan()" as the name for the peer discovery method. Semantic difference 
is minimal.

Rest is like for services but for a new object type: net.connman.peer

It's methods/signal are pretty straight forward.
About the properties, I am not fully sure we want to expose as many levels for 
its State.
GroupName is just a proposal as well, we could forget it right now. The idea 
behind is user
might need some extra info to recognize which peer to connect to, so peer name 
+ group name
would sound relevant.

Once this first step is over, we will be able to move on with these:

- Addressing services (discovery or providing ones)
- Group related issues (I think we can be smart and hide most of it frome the 
API)


Tomasz Bursztyka (1):
  doc: Add API documentation about WiFi P2P Peer

 doc/manager-api.txt| 29 +
 doc/peer-api.txt   | 49 +
 doc/technology-api.txt |  4 
 3 files changed, 82 insertions(+)
 create mode 100644 doc/peer-api.txt

-- 
1.8.3.2

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH] doc: Add API documentation about WiFi P2P Peer

[Tomasz Bursztyka]

If available, the p2p technology will be instanciated. Note that it's
fully dependant over wifi technology: wifi is the parent technology of
p2p.

If present, it will be possible to run a p2p peers discovery through p2p
technology's Scan() method. Results will be provided through Manager API
via GetPeers() method and/or PeeersChanged() signal.
---
 doc/manager-api.txt| 29 +
 doc/peer-api.txt   | 49 +
 doc/technology-api.txt |  4 
 3 files changed, 82 insertions(+)
 create mode 100644 doc/peer-api.txt

diff --git a/doc/manager-api.txt b/doc/manager-api.txt
index 8f1333c..445dd92 100644
--- a/doc/manager-api.txt
+++ b/doc/manager-api.txt
@@ -39,6 +39,13 @@ Methods  dict GetProperties()
 
Possible Errors: [service].Error.InvalidArguments
 
+   array{object,dict} GetPeers()
+
+   Returns a sorted list of tuples with peer object path
+   and dictionary of peer properties
+
+   Possible Errors: [peer].Error.InvalidArguments
+
object ConnectProvider(dict provider)   [deprecated]
 
Connect to a VPN specified by the given provider
@@ -186,6 +193,28 @@ SignalsTechnologyAdded(object path, dict 
properties)
required to watch the PropertyChanged signal of
the service object.
 
+   PeersChanged(array{object, dict})
+
+   Signals a list of peers that have been changed via the
+   first array. And a list of peer that have been removed
+   via the second array.
+
+   The list of changed peers is sorted. The dictionary
+   with the properties might be empty in case none of the
+   properties have changed. Or only contains the
+   properties that have changed.
+
+   For newly added peers the whole set of properties will
+   be present.
+
+   The list of removed peers can be empty.
+
+   This signal will only be triggered when the sort order
+   of the peer list or the number of peers changes. It
+   will not be emitted if only a property of the peer
+   object changes. For that it is required to watch the
+   PropertyChanged signal of the peer object.
+
PropertyChanged(string name, variant value)
 
This signal indicates a changed value of the given
diff --git a/doc/peer-api.txt b/doc/peer-api.txt
new file mode 100644
index 000..f148f1d
--- /dev/null
+++ b/doc/peer-api.txt
@@ -0,0 +1,49 @@
+Peer hierarchy [EXPERIMENTAL]
+=
+
+Servicenet.connman
+Interface  net.connman.Peer
+Object path[variable prefix]/{peer0,peer1,...}
+
+Methodsvoid Connect()
+
+   Connect this peer.
+
+   This method call will only return in case of an error
+   or when the peer is fully connected. So setting a
+   longer D-Bus timeout might be a really good idea.
+
+   Possible Errors: [service].Errori.InvalidArguments
+
+   void Disconnect()
+
+   Disconnect this peer. If the peer is not connected, an
+   error message will be generated.
+
+   Possible Errors: [service].Error.InvalidArguments
+
+SignalsPropertyChanged(string name, variant value)
+
+   This signal indicates a changed value of the given
+   property.
+
+Properties string State [readonly]
+
+   The peer state information.
+
+   Valid state are "idle", "failure", "association",
+   "configuration", "ready" and "disconnect".
+
+   string Name [readonly]
+
+   Name of the peer.
+
+   dict IPv4 [readonly]
+
+   string Address [readonly]
+
+   The current configured IPv4 address.
+
+   string Netmask [readonly]
+
+   The current configured IPv4 netmask.
diff --git a/doc/technology-api.txt b/doc/technology-api.txt
index 2da08de..77a0f02 100644
--- a/doc/technology-api.txt
+++ b/doc/technology-api.txt
@@ -36,6 +36,10 @@ Methods  dict GetProperties()  [deprecated]
Results will be signaled via the ServicesChanged
signal from the manager interface.
 
+   In case of P2P technology, results will be signaled
+   via the PeersChanged signal from the manager
+  

Re: [PATCH] doc: Add API documentation about WiFi P2P Peer

[Tomasz Bursztyka]

Hi Patrik,

Noticed 2 other unnecessary details to remove:


+
+Servicenet.connman
+Interface  net.connman.Peer
+Object path[variable prefix]/{peer0,peer1,...}
+
+Methodsvoid Connect()
+
+   Connect this peer.
+
+   This method call will only return in case of an error
+   or when the peer is fully connected. So setting a
+   longer D-Bus timeout might be a really good idea.
+
+   If we are already part of a group as the group owner,
+   it will invite this peer.


This detail on what's going to happen behind (about being a GO) is also 
superfluous



+
+   void Disconnect()
+
+   Disconnect this peer. If the peer is not connected, an
+   error message will be generated.
+
+   If we are already part of a group as the group owner,
+   it will reject this peer.



Same here.

I'll prepare a second version

Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[RFC] WiFi P2P Peer API - v1

[Tomasz Bursztyka]

Hi,

So here is the first shot for a P2P API propasl.

For a start, let's focus on the simplest case first: Peer client, in a 1 to 1 
context.

The idea is to create a new technology, whose hierarchical parent will be wifi 
technology.
(Thus if wifi is disabled, p2p is disabled. The other way round is not true) 

I kept "Scan()" as the name for the peer discovery method. Semantic difference 
is minimal.

Rest is like for services but for a new object type: net.connman.peer

It's methods/signal are pretty straight forward.
About the properties, I am not fully sure we want to expose as many levels for 
its State.
GroupName is just a proposal as well, we could forget it right now. The idea 
behind is user
might need some extra info to recognize which peer to connect to, so peer name 
+ group name
would sound relevant.

Once this first step is over, we will be able to move on with these:

- Addressing services (discovery or providing ones)
- Group related issues (I think we can be smart and hide most of it frome the 
API)

Tomasz Bursztyka (1):
  doc: Add API documentation about WiFi P2P Peer

 doc/manager-api.txt| 29 
 doc/peer-api.txt   | 60 ++
 doc/technology-api.txt |  5 +
 3 files changed, 94 insertions(+)
 create mode 100644 doc/peer-api.txt

-- 
1.8.3.2

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH] doc: Add API documentation about WiFi P2P Peer

[Tomasz Bursztyka]

If available, the p2p technology will be instanciated. Note that it's
fully dependant over wifi technology: wifi is the parent technology of
p2p.

If present, it will be possible to run a p2p peers discovery through p2p
technology's Scan() method. Results will be provided through Manager API
via GetPeers() method and/or PeeersChanged() signal.
---
 doc/manager-api.txt| 29 
 doc/peer-api.txt   | 60 ++
 doc/technology-api.txt |  5 +
 3 files changed, 94 insertions(+)
 create mode 100644 doc/peer-api.txt

diff --git a/doc/manager-api.txt b/doc/manager-api.txt
index 8f1333c..445dd92 100644
--- a/doc/manager-api.txt
+++ b/doc/manager-api.txt
@@ -39,6 +39,13 @@ Methods  dict GetProperties()
 
Possible Errors: [service].Error.InvalidArguments
 
+   array{object,dict} GetPeers()
+
+   Returns a sorted list of tuples with peer object path
+   and dictionary of peer properties
+
+   Possible Errors: [peer].Error.InvalidArguments
+
object ConnectProvider(dict provider)   [deprecated]
 
Connect to a VPN specified by the given provider
@@ -186,6 +193,28 @@ SignalsTechnologyAdded(object path, dict 
properties)
required to watch the PropertyChanged signal of
the service object.
 
+   PeersChanged(array{object, dict})
+
+   Signals a list of peers that have been changed via the
+   first array. And a list of peer that have been removed
+   via the second array.
+
+   The list of changed peers is sorted. The dictionary
+   with the properties might be empty in case none of the
+   properties have changed. Or only contains the
+   properties that have changed.
+
+   For newly added peers the whole set of properties will
+   be present.
+
+   The list of removed peers can be empty.
+
+   This signal will only be triggered when the sort order
+   of the peer list or the number of peers changes. It
+   will not be emitted if only a property of the peer
+   object changes. For that it is required to watch the
+   PropertyChanged signal of the peer object.
+
PropertyChanged(string name, variant value)
 
This signal indicates a changed value of the given
diff --git a/doc/peer-api.txt b/doc/peer-api.txt
new file mode 100644
index 000..c56ba8f
--- /dev/null
+++ b/doc/peer-api.txt
@@ -0,0 +1,60 @@
+Peer hierarchy [EXPERIMENTAL]
+=
+
+Servicenet.connman
+Interface  net.connman.Peer
+Object path[variable prefix]/{peer0,peer1,...}
+
+Methodsvoid Connect()
+
+   Connect this peer.
+
+   This method call will only return in case of an error
+   or when the peer is fully connected. So setting a
+   longer D-Bus timeout might be a really good idea.
+
+   If we are already part of a group as the group owner,
+   it will invite this peer.
+
+   Possible Errors: [service].Errori.InvalidArguments
+
+   void Disconnect()
+
+   Disconnect this peer. If the peer is not connected, an
+   error message will be generated.
+
+   If we are already part of a group as the group owner,
+   it will reject this peer.
+
+   Possible Errors: [service].Error.InvalidArguments
+
+SignalsPropertyChanged(string name, variant value)
+
+   This signal indicates a changed value of the given
+   property.
+
+Properties string State [readonly]
+
+   The peer state information.
+
+   Valid state are "idle", "failure", "association",
+   "configuration", "ready" and "disconnect".
+
+   string Name [readonly]
+
+   Name of the peer.
+
+   string GroupName [readonly]
+
+   Name of the group this peer belongs, as a group owner
+   or not.
+
+   dict IPv4 [readonly]
+
+   string Address [readonly]
+
+   The current configured IPv4 address.
+
+   string Netmask [readonly]
+
+   The current configured IPv4 netmask.
diff --git a/doc/technology-api.txt b/doc/technology-api.txt
index 2da08de..f82f0bb 100644
--- a/doc/techno

[PATCH] doc: Fix a small typo in technology documentation

[Tomasz Bursztyka]

---
 doc/technology-api.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/technology-api.txt b/doc/technology-api.txt
index 81adcbc..2da08de 100644
--- a/doc/technology-api.txt
+++ b/doc/technology-api.txt
@@ -45,7 +45,7 @@ Propertiesboolean Powered [readwrite]
 
Boolean representing the power state of the
technology. False means that the technology is
-   off (and is available RF-Killed) while True means
+   off (and if available: RF-Killed) while True means
that the technology is enabled.
 
boolean Connected [readonly]
-- 
1.8.3.2

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH] test: Add a script to test P2P through Wpa_supplicant DBus if

[Tomasz Bursztyka]

This script loosely mimics wpa_cli's P2P commands. It's just meant to
test wpa_supplicant's DBus API around P2P.
---
 test/p2p-on-supplicant | 311 +
 1 file changed, 311 insertions(+)
 create mode 100755 test/p2p-on-supplicant

diff --git a/test/p2p-on-supplicant b/test/p2p-on-supplicant
new file mode 100755
index 000..eefdad8
--- /dev/null
+++ b/test/p2p-on-supplicant
@@ -0,0 +1,311 @@
+#!/usr/bin/python
+
+from os import O_NONBLOCK
+from sys import stdin, stdout, exit, version_info
+from fcntl import fcntl, F_GETFL, F_SETFL
+import glib
+import dbus
+import dbus.mainloop.glib
+import gobject
+
+WPA_NAME='fi.w1.wpa_supplicant1'
+WPA_INTF='fi.w1.wpa_supplicant1'
+WPA_PATH='/fi/w1/wpa_supplicant1'
+WPA_IF_INTF = WPA_INTF + '.Interface'
+WPA_P2P_INTF = WPA_IF_INTF + '.P2PDevice'
+WPA_PEER_INTF = WPA_INTF + '.Peer'
+DBUS_PROPERTIES_INTF = 'org.freedesktop.DBus.Properties'
+
+class InputLine:
+def __init__(self, handler):
+self.line = ''
+self.handler = handler
+
+flags = fcntl(stdin.fileno(), F_GETFL)
+flags |= O_NONBLOCK
+fcntl(stdin.fileno(), F_SETFL, flags)
+glib.io_add_watch(stdin, glib.IO_IN, self.input_cb)
+
+self.prompt()
+
+def prompt(self):
+self.line = ''
+print '> ',
+stdout.flush()
+
+def input_cb(self, fd, event):
+if event != glib.IO_IN:
+return
+
+self.line += fd.read();
+for line in self.line.split('\n'):
+line = line.strip()
+if len(line) == 0:
+break
+
+self.handler(line.strip())
+
+self.prompt()
+
+return True
+
+def error_print(ex):
+print 'Command Error: %s' % ex
+
+def checkarg(nb_args):
+def under(function):
+def wrapper(*args, **kwargs):
+if len(args[1]) != nb_args:
+raise Exception('Command %s takes %s arguments' % 
(function.__name__, nb_args))
+return function(*args, **kwargs)
+return wrapper
+return under
+
+class Wpa_s:
+def __init__(self, iface_name = None):
+self.wpa = dbus.Interface(bus.get_object(WPA_NAME, WPA_PATH), WPA_INTF)
+bus.add_signal_receiver(self.__wpa_property_changed, path=WPA_PATH,
+member_keyword='signal')
+self.__reset()
+
+self.line_in = InputLine(self.__command)
+
+if self.iface_name != None:
+self.create_if(self.iface_name)
+
+def help(self, args):
+print 'Commands:'
+print 'quit'
+print 'create_if '
+print 'get_if '
+print 'del_if'
+print 'scan'
+print 'p2p_find'
+print 'p2p_stop_find'
+print 'p2p_flush'
+print 'p2p_group_add'
+print 'p2p_group_remove'
+print 'p2p_peers'
+print 'p2p_peer '
+print 'p2p_connect '
+
+def __command(self, cmd_line):
+cmd = cmd_line.split(' ')
+
+try:
+func = getattr(self, cmd[0])
+except Exception, e:
+print 'Error: command unknown - %s' % e
+return
+
+try:
+func(cmd[1:])
+except Exception, e:
+error_print(e)
+
+def __wpa_property_changed(*args, **kwargs):
+print 'WPA - Signal:  %s' % kwargs.get('signal')
+
+def __if_property_changed(*args, **kwargs):
+print 'IF - Signal:  %s' % kwargs.get('signal')
+
+def __p2p_property_changed(*args, **kwargs):
+signal = kwargs.get('signal')
+print 'IF P2P - Signal:  %s' % signal
+
+"""
+It should be: __DeviceFound(self, object_path, properties)
+wpa_supplicant's DBus API is buggy here:
+- no properties are given
+"""
+def __DeviceFound(self, object_path):
+self.peers[object_path] = None
+
+peer = bus.get_object(WPA_INTF, object_path)
+peer_if = dbus.Interface(peer, DBUS_PROPERTIES_INTF)
+
+self.peers[object_path] = peer_if.GetAll(WPA_PEER_INTF)
+
+def __DeviceLost(self, object_path):
+if object_path in self.peers:
+del self.peers[object_path]
+
+"""
+"Of course"... properties are not the group's properties,
+but a bunch of informations related to the group:
+- object path of the interface object
+- it's role
+- object path of the group object
+"""
+def __GroupStarted(self, properties):
+self.group = properties
+self.group_if = dbus.Interface(bus.get_object(WPA_INTF,
+self.group['interface_object']),
+   WPA_P2P_INTF)
+
+def __listen_if_signals(self):
+bus.add_signal_receiver(self.__if_property_changed,
+dbus_interface=WPA_IF_INTF,
+path=self.iface_path,
+member_keyword='signal')
+ 

[PATCH] test: Remove useless supplicant's old DBus interface test script

[Tomasz Bursztyka]

This DBus interface is no longer in use by ConnMan for some years
already.
---
 test/test-supplicant | 71 
 1 file changed, 71 deletions(-)
 delete mode 100755 test/test-supplicant

diff --git a/test/test-supplicant b/test/test-supplicant
deleted file mode 100755
index 68ac663..000
--- a/test/test-supplicant
+++ /dev/null
@@ -1,71 +0,0 @@
-#!/usr/bin/python
-
-import dbus
-import time
-
-WPA_NAME='fi.epitest.hostap.WPASupplicant'
-WPA_INTF='fi.epitest.hostap.WPASupplicant'
-WPA_PATH='/fi/epitest/hostap/WPASupplicant'
-
-bus = dbus.SystemBus()
-
-dummy = dbus.Interface(bus.get_object(WPA_NAME, WPA_PATH),
-   'org.freedesktop.DBus.Introspectable')
-
-#print dummy.Introspect()
-
-manager = dbus.Interface(bus.get_object(WPA_NAME, WPA_PATH), WPA_INTF)
-
-try:
-   path = manager.getInterface("wlan0")
-except:
-   path = manager.addInterface("wlan0")
-
-interface = dbus.Interface(bus.get_object(WPA_NAME, path),
-   WPA_INTF + ".Interface")
-
-print "state = %s" % (interface.state())
-
-try:
-   print "scanning = %s" % (interface.scanning())
-except:
-   pass
-
-print "[ %s ]" % (path)
-
-capabilities = interface.capabilities()
-
-for key in capabilities.keys():
-   list = ""
-   for value in capabilities[key]:
-   list += " " + value
-   print "%s =%s" % (key, list)
-
-interface.scan()
-
-time.sleep(1)
-
-try:
-   print "scanning = %s" % (interface.scanning())
-except:
-   pass
-
-time.sleep(1)
-
-print "state = %s" % (interface.state())
-
-results = interface.scanResults()
-
-print results
-
-path = results[0]
-
-print "[ %s ]" % (path)
-
-bssid = dbus.Interface(bus.get_object(WPA_NAME, path),
-   WPA_INTF + ".BSSID")
-
-properties = bssid.properties()
-
-for key in properties.keys():
-   print "%s = %s" % (key, properties[key])
-- 
1.8.3.2

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH v0 0/3] Miscellaneous session patches

[Tomasz Bursztyka]

Hi Daniel,


   #3 session: Use strings as bearer type

  Instead using the service type in session everywhere we just use
  the bearer string. With that we don't need the combersome
  connman_session_parse_bearers() anymore. A nice side effect is also
  less code:)


Less code, but more memory (every session holding their own string list 
of bearers) and (a bit) slower computation.


The cumbersome parser logic could have been improved to get best of both no?

Tomasz



___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH] WiFi Direct: WiFi Direct(WiFi P2P) RFC

[Tomasz Bursztyka]

Hi Martin,

We just started to take a look at P2P also.

My comments below anyway:


The API define references the BlueZ Adapter and Device API, WiFi Direct device
like BlueZ Adapter represents the local WiFi Direct device. WiFi Direct peer 
like
BlueZ Device represents the remote WiFi Direct peer.


You can't really mimic how bluez is behaving here.
Connman does not expose any device, since it's does not handle any directly
(a daemon below does that: bluez, wpa_supplicant...)

Connman will expose a P2P technology.



Dbus object Manager will be used.

WiFi Direct device object /net/connman/p2p0(WiFi Direct device interface)

WiFi Direct peer Objects scaned by the /net/connman/p2p0 is
/net/connman/p2p0/peer_xx_xx_xx_xx_xx_xx
---
  doc/wifi-direct-api.txt |  150 +++
  1 file changed, 150 insertions(+)
  create mode 100644 doc/wifi-direct-api.txt

diff --git a/doc/wifi-direct-api.txt b/doc/wifi-direct-api.txt
new file mode 100644
index 000..2105af4
--- /dev/null
+++ b/doc/wifi-direct-api.txt
@@ -0,0 +1,150 @@
+WiFiDirectDevice hierarchy
+
+
+Servicenet.connman
+Interface  net.connman.WiFiDirectDevice
+Object path[variable prefix]/{p2p0, p2p1,...}
+
+Methodsvoid Scan()
+
+   Scan WiFi Direct groups and devices.


For instance Scan() should be part of P2P technology.
And it will find out peers, not devices.

Such peers should be announced through signals, with there properties etc...


+
+   void Disconnect()
+
+   Disconnect group client from the WiFi Direct group.
+   Valid only when WiFi Direct device is group client.
+
+   void DeactivateGroup()
+
+   Disconnect all the group clients and terminate
+   the group session.
+   Valid only when WiFi Direct device is group owner.
+
+void CreateTempGroup()
+
+   Create temporary group, and activate the group by
+   starting the group session. The WiFi Direct device
+   works as the group owner.
+
+void CreateGroup(string name)
+
+   Create persistant group, and activate the group by
+   starting the group session. The WiFi Direct device
+   works as the group owner.
+
+void LoadGroup(string name)
+   Load the persistand group created before, and activeate
+   the group by starting the group session.
+
+void RemoveGroup(string name)
+
+Remove a persistant group. And the group can't be 
loaded
+any more.
+
+   array{string} GetGroupList()
+Get the persistant group list.


What are the proper use case to be fixed here with these methods?
Looks like very low level to me. This P2P group feature should be well 
thought.



+
+
+Properties boolean Powered [readwrite]
+
+   The power state of WiFi Direct device.


We don't care of knowing that since we are not device oriented


+
+   string Name [readwrite]
+
+   WiFi Direct device name


You mean the peer name it will expose? Connman can be smart and use the 
hostname or so...



+
+   uint16 Type [readwrite]
+
+   WiFi Direct device type


Which means what?
And usually, in Connman, if we have to expose such info we make it human 
readable (see Service type for instance)

That said, is it a useful info here?


+
+   string WPSmode[readwrite]
+
+   WPS configure mode (for example "pbc, pin" etc.)
+
+   uint8 Intent[readwrite]
+
+   Group owner intent value, valid value is [0-15]


Exposing this raises some questions too. It's blindly exposing a feature 
that might not be relevant for apps.
Currently I don't see why apps would care tweaking that. I might be 
missing something here though.
Actually, I don't get the point of this intent stuff: either you want to 
be a GO or then not.

Here it's like: why not letting the peer telling "Maybe yes, maybe not!" ;)


+
+boolean Connected [readonly]
+
+   Indicates whether the WiFi Direct device is connected.
+
+string GroupName[readonly]
+
+   Group name the WiFi Direct device belongs to.
+
+   string GroupState [readonly]
+
+   Persistant
+   Temporary
+   Client
+   Deactive


What's the purpose?


+
+   array{object} GroupMembers [readonly]
+
+   Group memeber list
+
+   string Passphrase[readonly]
+
+   Passphrase to join the WiFi Direct group.
+   

[Pacrunner PATCH 2/3] unit: FTP comes before SOCKS* in protocol order

[Tomasz Bursztyka]

Quick fix of "Manual configuration with exclusion" test, result was good
but in wrong order from protocol point of view.
---
 unit/suite/manual_exclude.test | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/unit/suite/manual_exclude.test b/unit/suite/manual_exclude.test
index ab35bf0..c155743 100644
--- a/unit/suite/manual_exclude.test
+++ b/unit/suite/manual_exclude.test
@@ -24,7 +24,7 @@ DIRECT
 ftp://test.foo.org test.foo.org
 DIRECT
 http://proxied.org proxied.org
-PROXY proxy.internal.com; PROXY secproxy.internal.com; SOCKS4 
sockproxy.internal.com; PROXY allftp.internal.com
+PROXY proxy.internal.com; PROXY secproxy.internal.com; PROXY 
allftp.internal.com; SOCKS4 sockproxy.internal.com
 https://bar.net/?property=true bar.net
 DIRECT
 trivial.co.uk:2984 trivial.co.uk
-- 
1.8.3.2

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[Pacrunner PATCH 3/3] plugins: Do not try to resolve a wrongly formated hostname

[Tomasz Bursztyka]

This will prevent to uselessly resolve a node which is not a valid
hostname: when the node is an IPv6 address for instance. In order not to
eat too much time when resolving a node.

Note:
Current PAC API is IPv4 only. For both IPv4/IPv6 support, a new API has
been proposed by Microsoft (look for FindProxyforURLEx). Though one
might argue it could have been better to fix legacy function rather than
inventing new ones, this patch should not affect any later
implementation of such API, like dnsResolveEx()/isInNetEx() etc...

Reported by David Woodhouse 
---
 plugins/mozjs.c | 11 +++
 plugins/v8.cc   | 14 +-
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/plugins/mozjs.c b/plugins/mozjs.c
index 9d1ad6f..af91fea 100644
--- a/plugins/mozjs.c
+++ b/plugins/mozjs.c
@@ -122,9 +122,20 @@ static JSBool dnsresolve(JSContext *ctx, uintN argc, jsval 
*vp)
char address[NI_MAXHOST];
jsval *argv = JS_ARGV(ctx, vp);
char *host = JS_EncodeString(ctx, JS_ValueToString(ctx, argv[0]));
+   char **split_res;
 
DBG("host %s", host);
 
+   /* Q&D test on host to know if it is a proper hostname */
+   split_res = g_strsplit_set(host, ":%?!,;@\\'*|<>{}[]()+=$&~# \"", -1);
+   if (split_res) {
+   int length = g_strv_length(split_res);
+   g_strfreev(split_res);
+
+   if (length > 1)
+   goto out;
+   }
+
JS_SET_RVAL(ctx, vp, JSVAL_NULL);
 
if (resolve(host, address, sizeof(address)) < 0)
diff --git a/plugins/v8.cc b/plugins/v8.cc
index cc38189..6cb28ca 100644
--- a/plugins/v8.cc
+++ b/plugins/v8.cc
@@ -123,13 +123,25 @@ static v8::Handle dnsresolve(const 
v8::Arguments& args)
 {
char address[NI_MAXHOST];
v8::String::Utf8Value host(args[0]);
+   char **split_res;
 
if (args.Length() != 1)
return v8::ThrowException(v8::String::New("Bad parameters"));
 
-   
DBG("host %s", *host);
 
+   /* Q&D test on host to know if it is a proper hostname */
+   split_res = g_strsplit_set(*host, ":%?!,;@\\'*|<>{}[]()+=$&~# \"", -1);
+   if (split_res) {
+   int length = g_strv_length(split_res);
+   g_strfreev(split_res);
+
+   if (length > 1) {
+   v8::ThrowException(
+   v8::String::New("Failed to resolve"));
+   }
+   }
+
if (resolve(*host, address, sizeof(address)) < 0)
return v8::ThrowException(v8::String::New("Failed to resolve"));
 
-- 
1.8.3.2

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[Pacrunner PATCH 0/3] Minor build/test fixes + ns resolution made faster

[Tomasz Bursztyka]

First 2 patches are very minor fixes for build process and unit test.

Third one is about not resolving badly formated hostname. This is relevant an 
many case,
especially if the given host is an IPv6 address as it can occur many times in 
IPv6 environment:
current PAC js API is not IPv6 oriented... so dnsResolve() should not try to 
resolve such addresses (among wrong hostnames)

Tomasz Bursztyka (3):
  build: Undefine _FORTIFY_SOURCE before redefining it
  unit: FTP comes before SOCKS* in protocol order
  plugins: Do not try to resolve a wrongly formated hostname

 acinclude.m4   |  2 +-
 plugins/mozjs.c| 11 +++
 plugins/v8.cc  | 14 +-
 unit/suite/manual_exclude.test |  2 +-
 4 files changed, 26 insertions(+), 3 deletions(-)

-- 
1.8.3.2

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[Pacrunner PATCH 1/3] build: Undefine _FORTIFY_SOURCE before redefining it

[Tomasz Bursztyka]

This prevents such annoying warning:

:0:0: warning: "_FORTIFY_SOURCE" redefined [enabled by
default]
---
 acinclude.m4 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 329c6a9..51630a8 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -12,7 +12,7 @@ AC_DEFUN([AC_PROG_CC_PIE], [
 
 AC_DEFUN([COMPILER_FLAGS], [
if (test "${CFLAGS}" = ""); then
-   CFLAGS="-Wall -O2 -D_FORTIFY_SOURCE=2"
+   CFLAGS="-Wall -O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2"
fi
if (test "$USE_MAINTAINER_MODE" = "yes"); then
CFLAGS+=" -Werror -Wextra"
-- 
1.8.3.2

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: Connman integration with PPPD?

[Tomasz Bursztyka]

Hi Glenn,


I'm currently using 1.19 with plans to move forward. Another question
I have is whether it's possible to tell Connman*not*  to manage a
particular interface (e.g. perhaps another service manages it
instead)? Is there a way to configure Connman to ignore specific
technologies or concrete interfaces of an otherwise managed
technology? Sort of like an interface "blacklist".


For this, either use connmand option -I or connman's main.conf 
NetworkInterfaceBlacklist key ( /etc/connman/main.conf )


Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH] device: Security should be provided when scanning for a hidden SSID

[Tomasz Bursztyka]

This will permit to ensure user connects the right network, when
connecting for the first time to a hidden network: current issues is
ConnMan does not check requested hidden network security when
connecting to it, thus allowing connect attemps to wrong network(s).

This patch also apply the same scan() function signature all over
the place (gpointer to void *, for the user_data part).

Reported by Alexandru Costache
---
 include/device.h |  2 +-
 plugins/wifi.c   | 11 +++
 src/connman.h|  2 +-
 src/device.c | 10 +-
 src/service.c|  5 -
 5 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/include/device.h b/include/device.h
index c14c78d..9243ce4 100644
--- a/include/device.h
+++ b/include/device.h
@@ -127,7 +127,7 @@ struct connman_device_driver {
int (*scan)(struct connman_device *device,
const char *ssid, unsigned int ssid_len,
const char *identity, const char* passphrase,
-   void *user_data);
+   const char *security, void *user_data);
int (*set_regdom) (struct connman_device *device,
const char *alpha2);
 };
diff --git a/plugins/wifi.c b/plugins/wifi.c
index b820540..924c095 100644
--- a/plugins/wifi.c
+++ b/plugins/wifi.c
@@ -71,6 +71,7 @@ struct hidden_params {
unsigned int ssid_len;
char *identity;
char *passphrase;
+   char *security;
GSupplicantScanParams *scan_params;
gpointer user_data;
 };
@@ -586,6 +587,7 @@ static void hidden_free(struct hidden_params *hidden)
g_supplicant_free_scan_params(hidden->scan_params);
g_free(hidden->identity);
g_free(hidden->passphrase);
+   g_free(hidden->security);
g_free(hidden);
 }
 
@@ -1047,7 +1049,7 @@ static int wifi_scan_simple(struct connman_device *device)
 static int wifi_scan(struct connman_device *device,
const char *ssid, unsigned int ssid_len,
const char *identity, const char* passphrase,
-   gpointer user_data)
+   const char *security, void *user_data)
 {
struct wifi_data *wifi = connman_device_get_data(device);
GSupplicantScanParams *scan_params = NULL;
@@ -1118,6 +1120,7 @@ static int wifi_scan(struct connman_device *device,
hidden->ssid_len = ssid_len;
hidden->identity = g_strdup(identity);
hidden->passphrase = g_strdup(passphrase);
+   hidden->security = g_strdup(security);
hidden->user_data = user_data;
wifi->hidden = hidden;
 
@@ -1848,9 +1851,9 @@ static void network_added(GSupplicantNetwork 
*supplicant_network)
connman_network_set_group(network, group);
 
if (wifi->hidden && ssid) {
-   if (wifi->hidden->ssid_len == ssid_len &&
-   memcmp(wifi->hidden->ssid, ssid,
-   ssid_len) == 0) {
+   if (!g_strcmp0(wifi->hidden->security, security) &&
+   wifi->hidden->ssid_len == ssid_len &&
+   !memcmp(wifi->hidden->ssid, ssid, ssid_len)) {
connman_network_connect_hidden(network,
wifi->hidden->identity,
wifi->hidden->passphrase,
diff --git a/src/connman.h b/src/connman.h
index f18b2f3..54afe89 100644
--- a/src/connman.h
+++ b/src/connman.h
@@ -534,7 +534,7 @@ int __connman_device_request_scan(enum connman_service_type 
type);
 int __connman_device_request_hidden_scan(struct connman_device *device,
const char *ssid, unsigned int ssid_len,
const char *identity, const char *passphrase,
-   gpointer user_data);
+   const char *security, void *user_data);
 
 bool __connman_device_isfiltered(const char *devname);
 
diff --git a/src/device.c b/src/device.c
index b455f4e..e112d27 100644
--- a/src/device.c
+++ b/src/device.c
@@ -591,7 +591,7 @@ int connman_device_set_powered(struct connman_device 
*device,
device->scanning = false;
 
if (device->driver && device->driver->scan)
-   device->driver->scan(device, NULL, 0, NULL, NULL, NULL);
+   device->driver->scan(device, NULL, 0, NULL, NULL, NULL, NULL);
 
return 0;
 }
@@ -609,7 +609,7 @@ static int device_scan(struct connman_device *device)
if (!device->powered)
return -ENOLINK;
 
-   return device->driver->scan(device, NULL, 0, NULL, NULL, NULL);
+   return device->driver->scan(device, NULL, 0, NULL, NULL, NULL, NULL);
 }
 
 int __connman_device_disconnect(struct connman_device *device)
@@ -1144,7 +1144,7 @@ int __connman_device_request_scan(enum 
connman_service_type type)
 int __connman_device

Re: [PATCH] gweb: Handle proxies as addresses and hostnames

[Tomasz Bursztyka]

Thanks Sjoerd, ACK from me.

Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH v2 05/12] service: Move __connman_service_set_active_session() to session.c

[Tomasz Bursztyka]

Hi,

> "struct connman_service *service"  to __connman_service_auto_connect,
> and then ConnMan don't need to lookup the service from service_list again.
>
> I think it is possible to optimize the algorithm, kindly give me your
> advice, please.

No there is nothing to "optimize" here with session,
__connman_service_auto_connect() is an importance piece in service.c.

What if the service the session was connected through is not an
auto-connectable service? What is if does not fit with preferred tech?
etc...

One session cannot decide on its own what to do.

Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH v2 00/12] Reimplementation of session selection implementation

[Tomasz Bursztyka]

Hi,


For example:
iptables -t mangle -A OUTPUT -m owner 1.1 \
   -j MARK --set-mark 258

A APP creates a session, and uses the above rule to route the network.

In this APP, if I create a socket to access the internet, I don't
understand
how kernel knows this socket has created by user 1.1 , and can access the
internet.


Looks like you need to take a look at linux kernel internals, your 
question is far to broad.
Basically: in the kernel your process runs under a uid/gid, when it 
opens a socket it owns it
(as any kind of stuff it can manipulate, it's part of its ressource). 
The relationship is direct.
On network traffic itself it's more complicated than that: depends on 
network configuration, iptables and routing rules.
There is no such information telling directly "this process can access 
this network".


About Session API, the idea here is to ensure the output traffic of 
application gets routed to the proper network.
To quickly summarize, iptables cannot touch the packet at routing step: 
it can do things before or after (which is too late then).
Thus session use iptables to mark application's packet before routing 
(see init_firewall_session() ) so that in routing,
such mark will be used to route the packet to proper gateway. (see 
add_default_route() )


Tomasz

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH v2 05/12] service: Move __connman_service_set_active_session() to session.c

[Tomasz Bursztyka]

Hi,


Here session has known which service(session->service) it will connect.
I think we can modify "__connman_service_auto_connect()", and add a
parameter:
__connman_service_auto_connect(struct connman_service *service);


No, first here the session is disconnecting and secondary 
__connman_service_auto_connect() is tightly following the service list 
order and other factors: it's meant to be automatic.


Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH] gweb: Handle proxies as addresses and hostnames

[Tomasz Bursztyka]

Hi,


There is finally a bigger issue here: in case handle_result_address()
>(aka you got_address() function) is called at this place,
>it should not call call_result_func() at all.
>
>You might handle this case via an extra parameter to
>handle_result_address()  like bool from_resolv  (or a more relevant name
>if you have one)

I've solved this slightly differently by always bouncing through back
through the mainloop such that the callback is always called after
do_request has returned. This makes the behaviour, form the API users
perspective, the same for both cases.


Makes sense, it's a much better approach indeed.


 From 2730e4447e0124361239be1bcaa937cf969da772 Mon Sep 17 00:00:00 2001
From: Sjoerd Simons
Date: Mon, 6 Jan 2014 13:51:06 +0100
Subject: [PATCH] gweb: Handle proxies as addresses and hostnames

It seems the proxy handling code was initially written to only handle
proxies in the form of IP addresses. 38b75abddb5b changed that
implicitly by always doing a hostname lookup for the proxy address,
which fixes proxies given by hostname but breaks ip based proxy
configuration (as sending an A query to most DNS server for an ip
address gets you a result with no answers).

Clean up the confusion by not overloading the meaning of the address
field in web_session and skipping the DNS lookup iff either the
proxy host or the real host is an ip address (as applicable).

Signed-off-by: Sjoerd Simons
---
  gweb/gweb.c | 87 +++--
  1 file changed, 61 insertions(+), 26 deletions(-)

diff --git a/gweb/gweb.c b/gweb/gweb.c
index 8cdd5d9..1955a85 100644
--- a/gweb/gweb.c
+++ b/gweb/gweb.c
@@ -68,6 +68,7 @@ struct web_session {
  
  	char *address;

char *host;
+   char *proxy_host;


Seriously it's uselessly bloating the structure. Use address as usual, 
or if the semantic really bothers you: rename address to something more 
relevant.
address has always been just "an intermediate variable" here, let's not 
add an extra one. There is no confusion here.



uint16_t port;
unsigned long flags;
struct addrinfo *addr;
@@ -79,6 +80,7 @@ struct web_session {
guint send_watch;
  
  	guint resolv_action;

+   guint idle_action;
char *request;
  
  	guint8 *receive_buffer;

@@ -162,6 +164,10 @@ static void free_session(struct web_session *session)
g_free(session->request);
  
  	web = session->web;

+
+   if (session->idle_action != 0)


Keep using > 0 as all the other guint.


+   g_source_remove(session->idle_action);
+
if (session->resolv_action > 0)
g_resolv_cancel_lookup(web->resolv, session->resolv_action);
  
@@ -190,6 +196,7 @@ static void free_session(struct web_session *session)

g_free(session->content_type);
  
  	g_free(session->host);

+   g_free(session->proxy_host);
g_free(session->address);
if (session->addr)
freeaddrinfo(session->addr);
@@ -1190,27 +1197,20 @@ static int parse_url(struct web_session *session,
}
}
  
-	session->address = g_strdup(host);

+   session->proxy_host = g_strdup(host);
  
  	g_free(scheme);
  
  	return 0;

  }
  
-static void resolv_result(GResolvResultStatus status,

-   char **results, gpointer user_data)
+static void got_address(struct web_session *session)


Change got_address() to handle_resolved_address().


  {
-   struct web_session *session = user_data;
struct addrinfo hints;
char *port;
int ret;
  
-	if (!results || !results[0]) {

-   call_result_func(session, 404);
-   return;
-   }
-
-   debug(session->web, "address %s", results[0]);
+   debug(session->web, "address %s", session->address);
  
  	memset(&hints, 0, sizeof(struct addrinfo));

hints.ai_flags = AI_NUMERICHOST;
@@ -1222,15 +1222,13 @@ static void resolv_result(GResolvResultStatus status,
}
  
  	port = g_strdup_printf("%u", session->port);

-   ret = getaddrinfo(results[0], port, &hints, &session->addr);
+   ret = getaddrinfo(session->address, port, &hints, &session->addr);
g_free(port);
if (ret != 0 || !session->addr) {
call_result_func(session, 400);
return;
}
  
-	g_free(session->address);

-   session->address = g_strdup(results[0]);
call_route_func(session);
  
  	if (create_transport(session) < 0) {

@@ -1239,12 +1237,54 @@ static void resolv_result(GResolvResultStatus status,
}
  }
  
+static gboolean idle_got_address(gpointer data)


A better name would be nice. Like already_resolved() ?


+{
+   struct web_session *session = data;
+
+   session->idle_action = 0;
+   got_address(session);
+   return FALSE;
+}
+
+static void resolv_result(GResolvResultStatus status,
+   char **results, gpointer user_data)
+{
+   str

Re: [PATCH] wifi: Fix "Unable to connect with the WEP mode AP again"

[Tomasz Bursztyka]

Hi


Unable to connect with the WEP mode AP again with correct passphrase
if connecting the AP with wrong passphrase before.

Because the WEP has not 4way-handshake, ConnMan can't receive
the error message of the invalid key, and can't set
the passphrase to NULL when connection failed with wrong passphrase.


Commit message is wrong.
WEP is specified so you cannot tell the key was wrong.
And wpa_supplicant does not provide any "error message". You are just 
assuming key was wrong,
but it could be due to something totally different (weak signal, buggy 
AP, etc...)



---
  plugins/wifi.c | 16 
  src/service.c  |  4 +++-
  2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/plugins/wifi.c b/plugins/wifi.c
index 533d6df..58bcff2 100644
--- a/plugins/wifi.c
+++ b/plugins/wifi.c
@@ -1416,6 +1416,7 @@ static void disconnect_callback(int result, 
GSupplicantInterface *interface,
  static int network_disconnect(struct connman_network *network)
  {
struct connman_device *device = connman_network_get_device(network);
+   struct connman_service *service;
struct wifi_data *wifi;
int err;
  
@@ -1425,6 +1426,21 @@ static int network_disconnect(struct connman_network *network)

if (!wifi || !wifi->interface)
return -ENODEV;
  
+	if (connman_network_get_associating(network) == TRUE)

+   connman_network_set_error(network,
+   CONNMAN_NETWORK_ERROR_ASSOCIATE_FAIL);
+   else {
+   service = connman_service_lookup_from_network(network);
+
+   if (service != NULL &&
+   (__connman_service_is_connected_state(service,
+   CONNMAN_IPCONFIG_TYPE_IPV4) == FALSE &&
+   __connman_service_is_connected_state(service,
+   CONNMAN_IPCONFIG_TYPE_IPV6) == FALSE) &&
+   (connman_service_get_favorite(service) == FALSE))
+   __connman_service_set_passphrase(service, NULL);
+   }
+


network_disconnect is called in case the user disconnects explicitly.
You can't tell the association went bad here, you just don't know about 
it. You cannot override wpa_supplicant interface's state.
And it's not up to the wifi plugin so decide what to do with the 
passphrase. service owns this role.



connman_network_set_associating(network, false);
  
  	if (wifi->disconnecting)

diff --git a/src/service.c b/src/service.c
index 33cce14..6583dbf 100644
--- a/src/service.c
+++ b/src/service.c
@@ -5424,7 +5424,9 @@ int __connman_service_indicate_error(struct 
connman_service *service,
  
  	set_error(service, error);
  
-	if (service->error == CONNMAN_SERVICE_ERROR_INVALID_KEY)

+   if (service->error == CONNMAN_SERVICE_ERROR_INVALID_KEY ||
+   (service->favorite == FALSE &&
+   service->error == CONNMAN_SERVICE_ERROR_CONNECT_FAILED))
__connman_service_set_passphrase(service, NULL);
  
  	/*


___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH] gweb: Handle proxies as addresses and hostnames

[Tomasz Bursztyka]

+ got_address (session, host);
  } else {


Again, no extra space between function name and argument list.

When there is one call after the if(): you don't need to put { }, by 
the way.


But actually you might want to get a status returned from 
handle_result_address(),
or then even if it has sent back a 400 or 409 http code, do_request() 
will still return a successful code and that's not what you want in 
such case: do_request() should return 0 then. 


There is finally a bigger issue here: in case handle_result_address() 
(aka you got_address() function) is called at this place,

it should not call call_result_func() at all.

You might handle this case via an extra parameter to 
handle_result_address()  like bool from_resolv  (or a more relevant name 
if you have one)


Thus mixing both a returned status code and this you could do:

if (ret != 0 || !session->addr) {
call_result_func(session, 400);
return;
}


would become:

if (ret != 0 || !session->addr) {
ret = 400;
goto error;
}

(...)

error:
if (from_resolv)
call_result_func(session, ret);

return ret;



Something like that.


Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH] gweb: Handle proxies as addresses and hostnames

[Tomasz Bursztyka]

Hi,


It seems the proxy handling code was initially written to only handle
proxies in the form of IP addresses. 38b75abddb5b changed that
implicitly by always doing a hostname lookup for the proxy address,
which fixes proxies given by hostname but breaks ip based proxy
configuration (as sending an A query to most DNS server for an ip
address gets you a result with no answers).


Good catch!

And you are actually fixing another issue with is_ip_address(), see below.
So you could tell it in your commit message.


Some comments:

  
  	char *address;

char *host;
+   char *proxy_host;


No need of that extra field, keep address for proxy as usual.


uint16_t port;
unsigned long flags;
struct addrinfo *addr;
@@ -190,6 +191,7 @@ static void free_session(struct web_session *session)
g_free(session->content_type);
  
  	g_free(session->host);

+   g_free(session->proxy_host);
g_free(session->address);
if (session->addr)
freeaddrinfo(session->addr);
@@ -1190,27 +1192,20 @@ static int parse_url(struct web_session *session,
}
}
  
-	session->address = g_strdup(host);

+   session->proxy_host = g_strdup(host);
  
  	g_free(scheme);
  
  	return 0;

  }
  
-static void resolv_result(GResolvResultStatus status,

-   char **results, gpointer user_data)
+static void got_address (struct web_session *session, const char *address)


Remove the extra space between function name and parameter list.
Change the name to: handle_result_address()


  {
-   struct web_session *session = user_data;
struct addrinfo hints;
char *port;
int ret;
  
-	if (!results || !results[0]) {

-   call_result_func(session, 404);
-   return;
-   }
-
-   debug(session->web, "address %s", results[0]);
+   debug(session->web, "address %s", address);
  
  	memset(&hints, 0, sizeof(struct addrinfo));

hints.ai_flags = AI_NUMERICHOST;
@@ -1222,7 +1217,7 @@ static void resolv_result(GResolvResultStatus status,
}
  
  	port = g_strdup_printf("%u", session->port);

-   ret = getaddrinfo(results[0], port, &hints, &session->addr);
+   ret = getaddrinfo(address, port, &hints, &session->addr);
g_free(port);
if (ret != 0 || !session->addr) {
call_result_func(session, 400);
@@ -1230,7 +1225,8 @@ static void resolv_result(GResolvResultStatus status,
}
  
  	g_free(session->address);

-   session->address = g_strdup(results[0]);
+   session->address = g_strdup(address);
+
call_route_func(session);
  
  	if (create_transport(session) < 0) {

@@ -1239,12 +1235,42 @@ static void resolv_result(GResolvResultStatus status,
}
  }
  
+static void resolv_result(GResolvResultStatus status,

+   char **results, gpointer user_data)
+{
+   struct web_session *session = user_data;
+
+   if (!results || !results[0]) {
+   call_result_func(session, 404);
+   return;
+   }
+
+   got_address (session, results[0]);


Remove the extra space between function name and argument list.


+}
+
+bool is_ip_address(const char *host)


Make this function static.


+{
+   struct addrinfo hints;
+   struct addrinfo *addr;
+   int result;
+
+   memset(&hints, 0, sizeof(struct addrinfo));
+   hints.ai_flags = AI_NUMERICHOST;
+   addr = NULL;
+
+   result = getaddrinfo(host, NULL, &hints, &addr);
+   freeaddrinfo(addr);
+
+   return result == 0;
+}
+
  static guint do_request(GWeb *web, const char *url,
const char *type, GWebInputFunc input,
int fd, gsize length, GWebResultFunc func,
GWebRouteFunc route, gpointer user_data)
  {
struct web_session *session;
+   const gchar *host;
  
  	if (!web || !url)

return 0;
@@ -1260,7 +1286,7 @@ static guint do_request(GWeb *web, const char *url,
return 0;
}
  
-	debug(web, "address %s", session->address);

+   debug(web, "proxy host %s", session->proxy_host);


The message content is more relevant here indeed. Just use 
session->address as usual.



debug(web, "port %u", session->port);
debug(web, "host %s", session->host);
debug(web, "flags %lu", session->flags);
@@ -1301,19 +1327,12 @@ static guint do_request(GWeb *web, const char *url,
session->header_done = false;
session->body_done = false;
  
-	if (!session->address && inet_aton(session->host, NULL) == 0) {


So here you are fixing another issue while removing inet_aton():
this was tight to ipv4, even though gweb can specifically work against 
ipv6 or unspecified family.



-   session->resolv_action = g_resolv_lookup_hostname(web->resolv,
-   session->host, resolv_

Re: [PATCH] gsupplicant: A network ssid of length 0 is valid, it's an hidden one

[Tomasz Bursztyka]

Hi Daniel,


I enabled my brain, got the answer:)

brainless greetigs,


And guess who reviewed your patch and thought it was totally fine ;)

Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH] gsupplicant: A network ssid of length 0 is valid, it's an hidden one

[Tomasz Bursztyka]

In case of an hidden AP around, its ssid has a length of 0, but it is
obviously a valid network anyway.

Fixes such regression from commit 363393cfb1a5f95f8892f40662486c87b80d0091

Reported by Sameer Naik
---
 gsupplicant/supplicant.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c
index 23ea41a..559dfbc 100644
--- a/gsupplicant/supplicant.c
+++ b/gsupplicant/supplicant.c
@@ -853,7 +853,7 @@ const char 
*g_supplicant_network_get_security(GSupplicantNetwork *network)
 const void *g_supplicant_network_get_ssid(GSupplicantNetwork *network,
unsigned int *ssid_len)
 {
-   if (!network || network->ssid_len == 0) {
+   if (!network) {
*ssid_len = 0;
return NULL;
}
-- 
1.8.4.4

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH 1/4] Add option to disable rfkill operation

[Tomasz Bursztyka]

Hi,


ConnMan should provide the option to enable/disable rfkill operation.


Can you give an explanation for that?

If you don't want rfkill, then just don't enable its support in the kernel.

Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: moveBefore not moving services correctly

[Tomasz Bursztyka]

I don't think it's that piece of code.

Actually, when moving the service, the ordering is explicit among the 2 
services your are moving. See switch_default_service()


Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: moveBefore not moving services correctly

[Tomasz Bursztyka]

Hi Mateusz,

The state is not an issue: you wifi access has no internet acces so it 
cannot get online.
And only one service can be online at a time and this can be only the 
default service (the one at the top of the list).


So the behavior in 1.15 was nominal. The state of your wired service 
gets "downgraded" to ready, but if you move it again before wifi, it 
will regain online state.


Now what you are experiencing in 1.19 is a bug, looks like the wifi 
service does not become the default service (but wired one got already 
it's state downgraded).


Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH] built: Undefine _FORTIFY_SOURCE before redefining it

[Tomasz Bursztyka]

This prevents such annoying warning:

:0:0: warning: "_FORTIFY_SOURCE" redefined [enabled by default]
---
 acinclude.m4 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 0a59871..65118da 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -12,7 +12,7 @@ AC_DEFUN([AC_PROG_CC_PIE], [
 
 AC_DEFUN([COMPILER_FLAGS], [
if (test "${CFLAGS}" = ""); then
-   CFLAGS="-Wall -O2 -D_FORTIFY_SOURCE=2"
+   CFLAGS="-Wall -O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2"
fi
if (test "$USE_MAINTAINER_MODE" = "yes"); then
CFLAGS+=" -Werror -Wextra"
-- 
1.8.4.4

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH 2/2] nfacct: Don't setup nfacct until it's relevant to do so

[Tomasz Bursztyka]

This prevents to load nfacct/nfnetlink modules when not needed.
---
 src/connman.h |  2 --
 src/main.c|  1 -
 src/nfacct.c  | 20 +++-
 3 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/src/connman.h b/src/connman.h
index 5bb11eb..bf59dbf 100644
--- a/src/connman.h
+++ b/src/connman.h
@@ -986,6 +986,4 @@ int __connman_nfacct_disable(struct nfacct_context *ctx,
connman_nfacct_disable_cb_t cb,
void *user_data);
 
-
-int __connman_nfacct_init(void);
 void __connman_nfacct_cleanup(void);
diff --git a/src/main.c b/src/main.c
index 5eeade5..e795b52 100644
--- a/src/main.c
+++ b/src/main.c
@@ -656,7 +656,6 @@ int main(int argc, char *argv[])
 
__connman_ippool_init();
__connman_iptables_init();
-   __connman_nfacct_init();
__connman_firewall_init();
__connman_nat_init();
__connman_tethering_init();
diff --git a/src/nfacct.c b/src/nfacct.c
index 7b803bd..f67c2ba 100644
--- a/src/nfacct.c
+++ b/src/nfacct.c
@@ -30,7 +30,7 @@
 #include "src/shared/nfacct.h"
 #include "src/shared/util.h"
 
-static struct nfacct_info *nfacct;
+static struct nfacct_info *nfacct = NULL;
 
 struct nfacct_rule {
char *name;
@@ -188,13 +188,18 @@ int __connman_nfacct_enable(struct nfacct_context *ctx,
connman_nfacct_enable_cb_t cb,
void *user_data)
 {
-   struct cb_data *cbd;
+   struct cb_data *cbd = NULL;
struct nfacct_rule *rule;
GList *list;
unsigned int id;
 
DBG("");
 
+   if (!nfacct)
+   nfacct = nfacct_new();
+   if (!nfacct)
+   goto err;
+
for (list = ctx->rules; list; list = list->next) {
rule = list->data;
 
@@ -367,17 +372,6 @@ int __connman_nfacct_flush(connman_nfacct_flush_cb_t cb, 
void *user_data)
return -ECOMM;
 }
 
-int __connman_nfacct_init(void)
-{
-   DBG("");
-
-   nfacct = nfacct_new();
-   if (!nfacct)
-   return -ENOMEM;
-
-   return 0;
-}
-
 void __connman_nfacct_cleanup(void)
 {
nfacct_destroy(nfacct);
-- 
1.8.4.4

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH 1/2] firewall: Don't do anything with iptables until really necessary

[Tomasz Bursztyka]

This prevents to load all iptables modules even though it will never be
used.
---
 src/connman.h  |  1 +
 src/firewall.c | 21 -
 src/session.c  | 21 ++---
 3 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/src/connman.h b/src/connman.h
index 0e3ec47..5bb11eb 100644
--- a/src/connman.h
+++ b/src/connman.h
@@ -951,6 +951,7 @@ int __connman_firewall_add_rule(struct firewall_context 
*ctx,
const char *rule_fmt, ...);
 int __connman_firewall_enable(struct firewall_context *ctx);
 int __connman_firewall_disable(struct firewall_context *ctx);
+bool __connman_firewall_is_up(void);
 
 int __connman_firewall_init(void);
 void __connman_firewall_cleanup(void);
diff --git a/src/firewall.c b/src/firewall.c
index e4443ea..90c3d3c 100644
--- a/src/firewall.c
+++ b/src/firewall.c
@@ -57,6 +57,8 @@ struct firewall_context {
 
 static GSList *managed_tables;
 
+static bool firewall_is_up;
+
 static int chain_to_index(const char *chain_name)
 {
if (!g_strcmp0(builtin_chains[NF_IP_PRE_ROUTING], chain_name))
@@ -341,6 +343,8 @@ int __connman_firewall_enable(struct firewall_context *ctx)
goto err;
}
 
+   firewall_is_up = true;
+
return 0;
 
 err:
@@ -356,6 +360,11 @@ int __connman_firewall_disable(struct firewall_context 
*ctx)
return firewall_disable(g_list_last(ctx->rules));
 }
 
+bool __connman_firewall_is_up(void)
+{
+   return firewall_is_up;
+}
+
 static void iterate_chains_cb(const char *chain_name, void *user_data)
 {
GSList **chains = user_data;
@@ -417,7 +426,17 @@ static void flush_table(const char *table_name)
 
 static void flush_all_tables(void)
 {
-   /* Flush the tables ConnMan might have modified */
+   /* Flush the tables ConnMan might have modified
+* But do so if only ConnMan has done something with
+* iptables */
+
+   if (!g_file_test("/proc/net/ip_tables_names",
+   G_FILE_TEST_EXISTS | G_FILE_TEST_IS_REGULAR)) {
+   firewall_is_up = false;
+   return;
+   }
+
+   firewall_is_up = true;
 
flush_table("filter");
flush_table("mangle");
diff --git a/src/session.c b/src/session.c
index 3fca6d6..f80e168 100644
--- a/src/session.c
+++ b/src/session.c
@@ -37,7 +37,7 @@ static GHashTable *session_hash;
 static struct connman_session *ecall_session;
 static GSList *policy_list;
 static uint32_t session_mark = 256;
-static struct firewall_context *global_firewall;
+static struct firewall_context *global_firewall = NULL;
 
 enum connman_session_trigger {
CONNMAN_SESSION_TRIGGER_UNKNOWN = 0,
@@ -236,6 +236,9 @@ static int init_firewall(void)
struct firewall_context *fw;
int err;
 
+   if (global_firewall)
+   return 0;
+
fw = __connman_firewall_create();
 
err = __connman_firewall_add_rule(fw, "mangle", "INPUT",
@@ -281,6 +284,10 @@ static int init_firewall_session(struct connman_session 
*session)
 
DBG("");
 
+   err = init_firewall();
+   if (err < 0)
+   return err;
+
fw = __connman_firewall_create();
if (!fw)
return -ENOMEM;
@@ -2212,10 +2219,6 @@ int __connman_session_init(void)
 
DBG("");
 
-   err = init_firewall();
-   if (err < 0)
-   return err;
-
connection = connman_dbus_get_connection();
if (!connection)
return -1;
@@ -2223,14 +2226,18 @@ int __connman_session_init(void)
err = connman_notifier_register(&session_notifier);
if (err < 0) {
dbus_connection_unref(connection);
-   cleanup_firewall();
return err;
}
 
session_hash = g_hash_table_new_full(g_str_hash, g_str_equal,
NULL, cleanup_session);
 
-   __connman_nfacct_flush(session_nfacct_flush_cb, NULL);
+   if (__connman_firewall_is_up()) {
+   err = init_firewall();
+   if (err < 0)
+   return err;
+   __connman_nfacct_flush(session_nfacct_flush_cb, NULL);
+   }
 
return 0;
 }
-- 
1.8.4.4

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH v3 0/2] Improve firewall initialization

[Tomasz Bursztyka]

Same as before, with Patrik's comments applied.

Tomasz Bursztyka (2):
  firewall: Don't do anything with iptables until really necessary
  nfacct: Don't setup nfacct until it's relevant to do so

 src/connman.h  |  3 +--
 src/firewall.c | 21 -
 src/main.c |  1 -
 src/nfacct.c   | 20 +++-
 src/session.c  | 21 ++---
 5 files changed, 42 insertions(+), 24 deletions(-)

-- 
1.8.4.4

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH 1/2] firewall: Don't do anything with iptables until really necessary

[Tomasz Bursztyka]

Hi Patrik,


+   if (!global_firewall) {

Let's not poke on global_firewall outside of init_firewall(). Let's fix
this in init_firewall() instead, as global_firewall is internal to that
function. init_firewall() has a bug as it does not check if
global_firewall is set or not and sets it unconditionally every time -
and looses memory when doing so.


Indeed, will fix this too.


>+   err = init_firewall();
>+   if (!err)

Shouldn't this be if '(err < 0)' ?


It should yes...

Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH v2 0/2] Improve firewall initialization

[Tomasz Bursztyka]

Hi,

Got a version that works for both modules and built-in based iptables 
installation.

Tomasz Bursztyka (2):
  firewall: Don't do anything with iptables until really necessary
  nfacct: Don't setup nfacct until it's relevant to do so

 src/connman.h  |  3 +--
 src/firewall.c | 21 -
 src/main.c |  1 -
 src/nfacct.c   | 20 +++-
 src/session.c  | 20 +---
 5 files changed, 41 insertions(+), 24 deletions(-)

-- 
1.8.4.4

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH 1/2] firewall: Don't do anything with iptables until really necessary

[Tomasz Bursztyka]

This prevents to load all iptables modules even though it will never be
used.
---
 src/connman.h  |  1 +
 src/firewall.c | 21 -
 src/session.c  | 20 +---
 3 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/src/connman.h b/src/connman.h
index 0e3ec47..5bb11eb 100644
--- a/src/connman.h
+++ b/src/connman.h
@@ -951,6 +951,7 @@ int __connman_firewall_add_rule(struct firewall_context 
*ctx,
const char *rule_fmt, ...);
 int __connman_firewall_enable(struct firewall_context *ctx);
 int __connman_firewall_disable(struct firewall_context *ctx);
+bool __connman_firewall_is_up(void);
 
 int __connman_firewall_init(void);
 void __connman_firewall_cleanup(void);
diff --git a/src/firewall.c b/src/firewall.c
index e4443ea..90c3d3c 100644
--- a/src/firewall.c
+++ b/src/firewall.c
@@ -57,6 +57,8 @@ struct firewall_context {
 
 static GSList *managed_tables;
 
+static bool firewall_is_up;
+
 static int chain_to_index(const char *chain_name)
 {
if (!g_strcmp0(builtin_chains[NF_IP_PRE_ROUTING], chain_name))
@@ -341,6 +343,8 @@ int __connman_firewall_enable(struct firewall_context *ctx)
goto err;
}
 
+   firewall_is_up = true;
+
return 0;
 
 err:
@@ -356,6 +360,11 @@ int __connman_firewall_disable(struct firewall_context 
*ctx)
return firewall_disable(g_list_last(ctx->rules));
 }
 
+bool __connman_firewall_is_up(void)
+{
+   return firewall_is_up;
+}
+
 static void iterate_chains_cb(const char *chain_name, void *user_data)
 {
GSList **chains = user_data;
@@ -417,7 +426,17 @@ static void flush_table(const char *table_name)
 
 static void flush_all_tables(void)
 {
-   /* Flush the tables ConnMan might have modified */
+   /* Flush the tables ConnMan might have modified
+* But do so if only ConnMan has done something with
+* iptables */
+
+   if (!g_file_test("/proc/net/ip_tables_names",
+   G_FILE_TEST_EXISTS | G_FILE_TEST_IS_REGULAR)) {
+   firewall_is_up = false;
+   return;
+   }
+
+   firewall_is_up = true;
 
flush_table("filter");
flush_table("mangle");
diff --git a/src/session.c b/src/session.c
index 3fca6d6..42e3763 100644
--- a/src/session.c
+++ b/src/session.c
@@ -37,7 +37,7 @@ static GHashTable *session_hash;
 static struct connman_session *ecall_session;
 static GSList *policy_list;
 static uint32_t session_mark = 256;
-static struct firewall_context *global_firewall;
+static struct firewall_context *global_firewall = NULL;
 
 enum connman_session_trigger {
CONNMAN_SESSION_TRIGGER_UNKNOWN = 0,
@@ -281,6 +281,12 @@ static int init_firewall_session(struct connman_session 
*session)
 
DBG("");
 
+   if (!global_firewall) {
+   err = init_firewall();
+   if (!err)
+   return err;
+   }
+
fw = __connman_firewall_create();
if (!fw)
return -ENOMEM;
@@ -2212,10 +2218,6 @@ int __connman_session_init(void)
 
DBG("");
 
-   err = init_firewall();
-   if (err < 0)
-   return err;
-
connection = connman_dbus_get_connection();
if (!connection)
return -1;
@@ -2223,14 +2225,18 @@ int __connman_session_init(void)
err = connman_notifier_register(&session_notifier);
if (err < 0) {
dbus_connection_unref(connection);
-   cleanup_firewall();
return err;
}
 
session_hash = g_hash_table_new_full(g_str_hash, g_str_equal,
NULL, cleanup_session);
 
-   __connman_nfacct_flush(session_nfacct_flush_cb, NULL);
+   if (__connman_firewall_is_up()) {
+   err = init_firewall();
+   if (err < 0)
+   return err;
+   __connman_nfacct_flush(session_nfacct_flush_cb, NULL);
+   }
 
return 0;
 }
-- 
1.8.4.4

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH 2/2] nfacct: Don't setup nfacct until it's relevant to do so

[Tomasz Bursztyka]

This prevents to load nfacct/nfnetlink modules when not needed.
---
 src/connman.h |  2 --
 src/main.c|  1 -
 src/nfacct.c  | 20 +++-
 3 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/src/connman.h b/src/connman.h
index 5bb11eb..bf59dbf 100644
--- a/src/connman.h
+++ b/src/connman.h
@@ -986,6 +986,4 @@ int __connman_nfacct_disable(struct nfacct_context *ctx,
connman_nfacct_disable_cb_t cb,
void *user_data);
 
-
-int __connman_nfacct_init(void);
 void __connman_nfacct_cleanup(void);
diff --git a/src/main.c b/src/main.c
index 5eeade5..e795b52 100644
--- a/src/main.c
+++ b/src/main.c
@@ -656,7 +656,6 @@ int main(int argc, char *argv[])
 
__connman_ippool_init();
__connman_iptables_init();
-   __connman_nfacct_init();
__connman_firewall_init();
__connman_nat_init();
__connman_tethering_init();
diff --git a/src/nfacct.c b/src/nfacct.c
index 7b803bd..f67c2ba 100644
--- a/src/nfacct.c
+++ b/src/nfacct.c
@@ -30,7 +30,7 @@
 #include "src/shared/nfacct.h"
 #include "src/shared/util.h"
 
-static struct nfacct_info *nfacct;
+static struct nfacct_info *nfacct = NULL;
 
 struct nfacct_rule {
char *name;
@@ -188,13 +188,18 @@ int __connman_nfacct_enable(struct nfacct_context *ctx,
connman_nfacct_enable_cb_t cb,
void *user_data)
 {
-   struct cb_data *cbd;
+   struct cb_data *cbd = NULL;
struct nfacct_rule *rule;
GList *list;
unsigned int id;
 
DBG("");
 
+   if (!nfacct)
+   nfacct = nfacct_new();
+   if (!nfacct)
+   goto err;
+
for (list = ctx->rules; list; list = list->next) {
rule = list->data;
 
@@ -367,17 +372,6 @@ int __connman_nfacct_flush(connman_nfacct_flush_cb_t cb, 
void *user_data)
return -ECOMM;
 }
 
-int __connman_nfacct_init(void)
-{
-   DBG("");
-
-   nfacct = nfacct_new();
-   if (!nfacct)
-   return -ENOMEM;
-
-   return 0;
-}
-
 void __connman_nfacct_cleanup(void)
 {
nfacct_destroy(nfacct);
-- 
1.8.4.4

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[RFC 0/2] firewall/nfacct improvements

[Tomasz Bursztyka]

Hi,

It's been a while users, including myself, have noticed the big amount of 
iptables related modules
always being loaded at connman's startup. In a usage that, however, did not 
require those at all:
- no tethering usage
- no session usage

So here is a quick hack:
- let's not flush any connman's chain if the modules are not loaded (so it 
means we are on a fresh start and nothing, even connman, has been using 
iptables)
- same for nfacct, it initiates it when necessary only.

@Daniel: can you check it's not breaking you session/policy runtime logic?

Tomasz Bursztyka (2):
  firewall: Don't do anything with iptables until really necessary
  nfacct: Don't setup nfacct until it's relevant to do so

 src/connman.h  |  3 +--
 src/firewall.c | 21 -
 src/main.c |  1 -
 src/nfacct.c   | 20 +++-
 src/session.c  | 20 +---
 5 files changed, 41 insertions(+), 24 deletions(-)

-- 
1.8.4.3

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH 1/2] firewall: Don't do anything with iptables until really necessary

[Tomasz Bursztyka]

This prevents to load all iptables modules even though it will never be
used.
---
 src/connman.h  |  1 +
 src/firewall.c | 21 -
 src/session.c  | 20 +---
 3 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/src/connman.h b/src/connman.h
index 0e3ec47..5bb11eb 100644
--- a/src/connman.h
+++ b/src/connman.h
@@ -951,6 +951,7 @@ int __connman_firewall_add_rule(struct firewall_context 
*ctx,
const char *rule_fmt, ...);
 int __connman_firewall_enable(struct firewall_context *ctx);
 int __connman_firewall_disable(struct firewall_context *ctx);
+bool __connman_firewall_is_up(void);
 
 int __connman_firewall_init(void);
 void __connman_firewall_cleanup(void);
diff --git a/src/firewall.c b/src/firewall.c
index e4443ea..9d6846c 100644
--- a/src/firewall.c
+++ b/src/firewall.c
@@ -57,6 +57,8 @@ struct firewall_context {
 
 static GSList *managed_tables;
 
+static bool firewall_is_up;
+
 static int chain_to_index(const char *chain_name)
 {
if (!g_strcmp0(builtin_chains[NF_IP_PRE_ROUTING], chain_name))
@@ -341,6 +343,8 @@ int __connman_firewall_enable(struct firewall_context *ctx)
goto err;
}
 
+   firewall_is_up = true;
+
return 0;
 
 err:
@@ -356,6 +360,11 @@ int __connman_firewall_disable(struct firewall_context 
*ctx)
return firewall_disable(g_list_last(ctx->rules));
 }
 
+bool __connman_firewall_is_up(void)
+{
+   return firewall_is_up;
+}
+
 static void iterate_chains_cb(const char *chain_name, void *user_data)
 {
GSList **chains = user_data;
@@ -417,7 +426,17 @@ static void flush_table(const char *table_name)
 
 static void flush_all_tables(void)
 {
-   /* Flush the tables ConnMan might have modified */
+   /* Flush the tables ConnMan might have modified
+* But do so if only ConnMan has done something with
+* iptables */
+
+   if (!g_file_test("/sys/module/ip_tables",
+   G_FILE_TEST_EXISTS | G_FILE_TEST_IS_DIR)) {
+   firewall_is_up = false;
+   return;
+   }
+
+   firewall_is_up = true;
 
flush_table("filter");
flush_table("mangle");
diff --git a/src/session.c b/src/session.c
index 3fca6d6..42e3763 100644
--- a/src/session.c
+++ b/src/session.c
@@ -37,7 +37,7 @@ static GHashTable *session_hash;
 static struct connman_session *ecall_session;
 static GSList *policy_list;
 static uint32_t session_mark = 256;
-static struct firewall_context *global_firewall;
+static struct firewall_context *global_firewall = NULL;
 
 enum connman_session_trigger {
CONNMAN_SESSION_TRIGGER_UNKNOWN = 0,
@@ -281,6 +281,12 @@ static int init_firewall_session(struct connman_session 
*session)
 
DBG("");
 
+   if (!global_firewall) {
+   err = init_firewall();
+   if (!err)
+   return err;
+   }
+
fw = __connman_firewall_create();
if (!fw)
return -ENOMEM;
@@ -2212,10 +2218,6 @@ int __connman_session_init(void)
 
DBG("");
 
-   err = init_firewall();
-   if (err < 0)
-   return err;
-
connection = connman_dbus_get_connection();
if (!connection)
return -1;
@@ -2223,14 +2225,18 @@ int __connman_session_init(void)
err = connman_notifier_register(&session_notifier);
if (err < 0) {
dbus_connection_unref(connection);
-   cleanup_firewall();
return err;
}
 
session_hash = g_hash_table_new_full(g_str_hash, g_str_equal,
NULL, cleanup_session);
 
-   __connman_nfacct_flush(session_nfacct_flush_cb, NULL);
+   if (__connman_firewall_is_up()) {
+   err = init_firewall();
+   if (err < 0)
+   return err;
+   __connman_nfacct_flush(session_nfacct_flush_cb, NULL);
+   }
 
return 0;
 }
-- 
1.8.4.3

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH 2/2] nfacct: Don't setup nfacct until it's relevant to do so

[Tomasz Bursztyka]

This prevents to load nfacct/nfnetlink modules when not needed.
---
 src/connman.h |  2 --
 src/main.c|  1 -
 src/nfacct.c  | 20 +++-
 3 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/src/connman.h b/src/connman.h
index 5bb11eb..bf59dbf 100644
--- a/src/connman.h
+++ b/src/connman.h
@@ -986,6 +986,4 @@ int __connman_nfacct_disable(struct nfacct_context *ctx,
connman_nfacct_disable_cb_t cb,
void *user_data);
 
-
-int __connman_nfacct_init(void);
 void __connman_nfacct_cleanup(void);
diff --git a/src/main.c b/src/main.c
index 5eeade5..e795b52 100644
--- a/src/main.c
+++ b/src/main.c
@@ -656,7 +656,6 @@ int main(int argc, char *argv[])
 
__connman_ippool_init();
__connman_iptables_init();
-   __connman_nfacct_init();
__connman_firewall_init();
__connman_nat_init();
__connman_tethering_init();
diff --git a/src/nfacct.c b/src/nfacct.c
index 7b803bd..f67c2ba 100644
--- a/src/nfacct.c
+++ b/src/nfacct.c
@@ -30,7 +30,7 @@
 #include "src/shared/nfacct.h"
 #include "src/shared/util.h"
 
-static struct nfacct_info *nfacct;
+static struct nfacct_info *nfacct = NULL;
 
 struct nfacct_rule {
char *name;
@@ -188,13 +188,18 @@ int __connman_nfacct_enable(struct nfacct_context *ctx,
connman_nfacct_enable_cb_t cb,
void *user_data)
 {
-   struct cb_data *cbd;
+   struct cb_data *cbd = NULL;
struct nfacct_rule *rule;
GList *list;
unsigned int id;
 
DBG("");
 
+   if (!nfacct)
+   nfacct = nfacct_new();
+   if (!nfacct)
+   goto err;
+
for (list = ctx->rules; list; list = list->next) {
rule = list->data;
 
@@ -367,17 +372,6 @@ int __connman_nfacct_flush(connman_nfacct_flush_cb_t cb, 
void *user_data)
return -ECOMM;
 }
 
-int __connman_nfacct_init(void)
-{
-   DBG("");
-
-   nfacct = nfacct_new();
-   if (!nfacct)
-   return -ENOMEM;
-
-   return 0;
-}
-
 void __connman_nfacct_cleanup(void)
 {
nfacct_destroy(nfacct);
-- 
1.8.4.3

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH v0 04/13] inet: Set device name with null termination

[Tomasz Bursztyka]

Hi Jukka,


Either use IFNAMSIZ or sizeof(ifr.ifr_name) but let's not mix both.

I would prefer sizeof(), that way you do not need to know how long the
buffer is suppose to be.


I agree

Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH v0 10/13] ipv4ll: Initialize socket variable

[Tomasz Bursztyka]

Hi Daniel,



FWIW, in a couple of other functions memset is used. I do not really
care, but would like to have a consistent style. What's the preference? 


We need Marcel's input on that ;)

Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH v0 13/13] tap-test: Set device name with null termination

[Tomasz Bursztyka]

Hi Daniel,


-   strncpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name));
+   strncpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name) - 1);


Same as patch 4: IFNAMSIZ or these sizeof(), we have to choose 1 way.

Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH v0 12/13] dnsproxy-test: Close socket in error path

[Tomasz Bursztyka]

Hi Daniel,


+   close(sk);
err = -errno;


Invert these two lines, close() affects errno.

Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH v0 10/13] ipv4ll: Initialize socket variable

[Tomasz Bursztyka]

Hi Daniel,

struct sockaddr_ll sock;
+
fd = socket(PF_PACKET, SOCK_DGRAM | SOCK_CLOEXEC, htons(ETH_P_ARP));
if (fd < 0)
return fd;
  
+	memset(&sock, 0, sizeof(sock));


Or just struct sockaddr_ll sock = {};  so no need to call memset.

Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH v0 01/13] service: Remove dead code

[Tomasz Bursztyka]

Hi Daniel,


-   bool need_split = false;


That the error, not the rest. Jukka knows about that path, but looks 
like a bug to have this line in the for loop instead of externally.


Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH v0 04/13] inet: Set device name with null termination

[Tomasz Bursztyka]

Hi Daniel,


-   strncpy(ifr.ifr_name, tunnel, IFNAMSIZ);
+   strncpy(ifr.ifr_name, tunnel, IFNAMSIZ - 1);


Either use IFNAMSIZ or sizeof(ifr.ifr_name) but let's not mix both.

Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: APIPA regression in connman wired ethernet (bump)

[Tomasz Bursztyka]

Hi Tim,

Could you resend your logs and the exact steps you followed?

On my side I just tried: starting connman, plugging in ethernet cable... 
and it worked.

Are your steps different then that?

Here is the output:

connmand[14904]: Skipping disconnect of carrier, network is connecting.
connmand[14904]: 
src/service.c:__connman_service_ipconfig_indicate_state() service 
0x6ca780 (ethernet_82c40bd2dbda_cable) state 3 (configuration) type 1 (IPv4)
connmand[14904]: src/service.c:service_indicate_state() service 0x6ca780 
old idle - new configuration/idle => configuration

connmand[14904]: src/dhcp.c:__connman_dhcp_start()
connmand[14904]: src/dhcp.c:dhcp_request() dhcp 0x6c8220
connmand[14904]: eth0 {add} route ff00:: gw :: scope 0 
connmand[14904]: eth0 {add} route fe80:: gw :: scope 0 
connmand[14904]: eth0 {RX} 54761 packets 44944758 bytes
connmand[14904]: eth0 {TX} 42473 packets 6168781 bytes
connmand[14904]: eth0 {update} flags 102467 
connmand[14904]: eth0 {newlink} index 2 address 00:1F:16:11:0D:E5 mtu 1500
connmand[14904]: eth0 {newlink} index 2 operstate 6 
connmand[14904]: src/service.c:__connman_service_create_from_network() 
network 0x6d95f0

connmand[14904]: src/service.c:connman_service_create() service 0x6dd7e0
connmand[14904]: src/service.c:service_initialize() service 0x6dd7e0
connmand[14904]: src/service.c:service_get() service 0x6dd7e0
connmand[14904]: src/service.c:update_from_network() service 0x6dd7e0 
network 0x6d95f0

connmand[14904]: src/service.c:service_register() service 0x6dd7e0
connmand[14904]: src/service.c:service_register() path 
/net/connman/service/ethernet_001f16110de5_cable

connmand[14904]: src/service.c:service_load() service 0x6dd7e0
connmand[14904]: src/service.c:__connman_service_auto_connect()
connmand[14904]: src/service.c:service_schedule_added() service 0x6dd7e0
connmand[14904]: src/service.c:run_auto_connect()
connmand[14904]: src/service.c:auto_connect_service() preferred 0 sessions 0
connmand[14904]: src/service.c:service_send_changed()
connmand[14904]: src/service.c:service_append_added_foreach() changed 
/net/connman/service/ethernet_82c40bd2dbda_cable
connmand[14904]: src/service.c:service_append_added_foreach() new 
/net/connman/service/ethernet_001f16110de5_cable

connmand[14904]: src/dhcp.c:no_lease_cb() No lease available
connmand[14904]: src/dhcp.c:dhcp_invalidate() dhcp 0x6c8220 callback 0
connmand[14904]: src/dhcp.c:dhcp_invalidate() last address (null)
connmand[14904]: src/dhcp.c:ipv4ll_available_cb() IPV4LL available
connmand[14904]: src/service.c:__connman_service_get_order() service 
0x6ca780 name Wired order 1 split 0
connmand[14904]: src/service.c:connman_service_ref_debug() 0x6ca780 ref 
2 by src/connection.c:408:add_gateway()
connmand[14904]: src/service.c:__connman_service_indicate_default() 
service 0x6ca780 state configuration
connmand[14904]: 
src/service.c:__connman_service_ipconfig_indicate_state() service 
0x6ca780 (ethernet_82c40bd2dbda_cable) state 4 (ready) type 1 (IPv4)
connmand[14904]: src/service.c:service_rp_filter() connected 
ethernet_82c40bd2dbda_cable ipconfig 0x6d6610 method 4 count 1 filter 0
connmand[14904]: src/service.c:service_indicate_state() service 0x6ca780 
old configuration - new ready/idle => ready

connmand[14904]: src/service.c:default_changed() current default (nil)
connmand[14904]: src/service.c:default_changed() new default 0x6ca780 
ethernet_82c40bd2dbda_cable

connmand[14904]: src/service.c:service_save() service 0x6ca780 new 0
connmand[14904]: src/service.c:append_dns() append fallback nameservers
connmand[14904]: src/service.c:__connman_service_get_order() service 
0x6ca780 name Wired order 1 split 0
connmand[14904]: virtnic0 {add} address 169.254.68.155/16 label virtnic0 
family 2

connmand[14904]: src/service.c:service_ip_bound() virtnic0 ip bound
connmand[14904]: src/service.c:service_ip_bound() service 0x6ca780 
ipconfig 0x6d6610 type 1 method 4
connmand[14904]: virtnic0 {add} route 169.254.0.0 gw 0.0.0.0 scope 253 


connmand[14904]: virtnic0 {add} route 0.0.0.0 gw 0.0.0.0 scope 253 
connmand[14904]: src/service.c:__connman_service_indicate_default() 
service 0x6ca780 state ready

connmand[14904]: src/service.c:service_send_changed()
connmand[14904]: src/service.c:service_append_added_foreach() changed 
/net/connman/service/ethernet_82c40bd2dbda_cable
connmand[14904]: src/service.c:service_append_added_foreach() changed 
/net/connman/service/ethernet_001f16110de5_cable


Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: src/technology.c:technology_get() No matching drivers found for wifi

[Tomasz Bursztyka]

Hi Steve,

Do you have wpa_supplicant installed?
(and properly build: look at connman's README file to get the right 
configuration)


Connman's technology drivers are not literally hardware drivers: they 
are backend drivers
(wifi connman's driver is against wpa_supplicant, bluetooth connman's 
driver against bluez etc etc)


Br,

Tomasz

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: Cellular connection with oFono/ConnMan

[Tomasz Bursztyka]

Hi Yannick,

You need to ask ConnMan to connect the related cellular service that 
should show up in the service list.
What you are listing here is the supported technologies that ConnMan 
detected on your setup.


Try connmanctl tool, to list the service and to connect the relevant one.

Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH] wispr: Get the proper proxy address from the result

[Tomasz Bursztyka]

It has to strip away the "PROXY" word and spaces from the proxy lookup
result, or then gweb won't be able to resolve it.
---
 src/wispr.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/wispr.c b/src/wispr.c
index 7894537..32cf75a 100644
--- a/src/wispr.c
+++ b/src/wispr.c
@@ -775,8 +775,13 @@ static void proxy_callback(const char *proxy, void 
*user_data)
 
wp_context->token = 0;
 
-   if (proxy && g_strcmp0(proxy, "DIRECT") != 0)
+   if (proxy && g_strcmp0(proxy, "DIRECT") != 0) {
+   if (g_str_has_prefix(proxy, "PROXY")) {
+   proxy += 5;
+   for (; *proxy == ' ' && *proxy != '\0'; proxy++);
+   }
g_web_set_proxy(wp_context->web, proxy);
+   }
 
g_web_set_accept(wp_context->web, NULL);
g_web_set_user_agent(wp_context->web, "ConnMan/%s wispr", VERSION);
-- 
1.8.3.2

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH] gweb: Properly proceed a request through a proxy when one is set

[Tomasz Bursztyka]

This fixes a logical bug in gweb, as for a given host: if a proxy
address is provided it should resolv its DNS through g_resolv and not
getaddrinfo: g_resolv will then use the nameservers that have been set
to the g_web session.

Indirectly, this fixes a bug when ConnMan was always staying at ready
when service was proxied, even though such proxy would allow to go on
internet.
---
 gweb/gweb.c | 26 +++---
 1 file changed, 3 insertions(+), 23 deletions(-)

diff --git a/gweb/gweb.c b/gweb/gweb.c
index 3d9645a..81dbff0 100644
--- a/gweb/gweb.c
+++ b/gweb/gweb.c
@@ -1308,32 +1308,12 @@ static guint do_request(GWeb *web, const char *url,
return 0;
}
} else {
-   struct addrinfo hints;
-   char *port;
-   int ret;
-
if (!session->address)
session->address = g_strdup(session->host);
 
-   memset(&hints, 0, sizeof(struct addrinfo));
-   hints.ai_flags = AI_NUMERICHOST;
-   hints.ai_family = session->web->family;
-
-   if (session->addr) {
-   freeaddrinfo(session->addr);
-   session->addr = NULL;
-   }
-
-   port = g_strdup_printf("%u", session->port);
-   ret = getaddrinfo(session->address, port, &hints,
-   &session->addr);
-   g_free(port);
-   if (ret != 0 || !session->addr) {
-   free_session(session);
-   return 0;
-   }
-
-   if (create_transport(session) < 0) {
+   session->resolv_action = g_resolv_lookup_hostname(web->resolv,
+   session->address, resolv_result, 
session);
+   if (session->resolv_action == 0) {
free_session(session);
return 0;
}
-- 
1.8.3.2

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH] service: Proxy should be reset when disconnecting

[Tomasz Bursztyka]

Fixes an issue where WPAD is started only at first connection, not after
a disconnection: it takes for granted the previous proxy setting which
is bogus (since last wpad url has be nuked)
---
 src/service.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/service.c b/src/service.c
index a576f6d..7ff7339 100644
--- a/src/service.c
+++ b/src/service.c
@@ -5945,6 +5945,7 @@ int __connman_service_disconnect(struct connman_service 
*service)
DBG("service %p", service);
 
service->userconnect = false;
+   service->proxy = CONNMAN_SERVICE_PROXY_METHOD_UNKNOWN;
 
connman_agent_cancel(service);
 
-- 
1.8.3.2

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH 2/5] service: 32bits boolean should be used due to dbus in relevant places

[Tomasz Bursztyka]

---
 src/service.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/service.c b/src/service.c
index 288deb7..60a9769 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1412,15 +1412,18 @@ static void immutable_changed(struct connman_service 
*service)
 
 static void roaming_changed(struct connman_service *service)
 {
+   dbus_bool_t roaming;
+
if (!service->path)
return;
 
if (!allow_property_changed(service))
return;
 
+   roaming = service->roaming;
connman_dbus_property_changed_basic(service->path,
CONNMAN_SERVICE_INTERFACE, "Roaming",
-   DBUS_TYPE_BOOLEAN, &service->roaming);
+   DBUS_TYPE_BOOLEAN, &roaming);
 }
 
 static void autoconnect_changed(struct connman_service *service)
@@ -3062,7 +3065,7 @@ static DBusMessage *set_property(DBusConnection *conn,
type = dbus_message_iter_get_arg_type(&value);
 
if (g_str_equal(name, "AutoConnect")) {
-   bool autoconnect;
+   dbus_bool_t autoconnect;
 
if (type != DBUS_TYPE_BOOLEAN)
return __connman_error_invalid_arguments(msg);
-- 
1.8.3.2

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH 4/5] vpn: 32bits boolean should be used due to dbus in relevant places

[Tomasz Bursztyka]

---
 plugins/vpn.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/plugins/vpn.c b/plugins/vpn.c
index 14a6063..cc38890 100644
--- a/plugins/vpn.c
+++ b/plugins/vpn.c
@@ -605,7 +605,10 @@ static void add_connection(const char *path, 
DBusMessageIter *properties,
dbus_message_iter_get_basic(&value, &str);
data->type = g_strdup(str);
} else if (g_str_equal(key, "Immutable")) {
-   dbus_message_iter_get_basic(&value, &data->immutable);
+   dbus_bool_t immutable;
+
+   dbus_message_iter_get_basic(&value, &immutable);
+   data->immutable = immutable;
} else if (g_str_equal(key, "Host")) {
dbus_message_iter_get_basic(&value, &str);
data->host = g_strdup(str);
-- 
1.8.3.2

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH 3/5] ofono: 32bits boolean should be used due to dbus in relevant places

[Tomasz Bursztyka]

---
 plugins/ofono.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/plugins/ofono.c b/plugins/ofono.c
index 939cd7f..d791b1b 100644
--- a/plugins/ofono.c
+++ b/plugins/ofono.c
@@ -1066,7 +1066,7 @@ static int add_cm_context(struct modem_data *modem, const 
char *context_path,
 {
const char *context_type = NULL;
struct network_context *context = NULL;
-   bool active = false;
+   dbus_bool_t active = FALSE;
 
DBG("%s context path %s", modem->path, context_path);
 
@@ -1698,7 +1698,10 @@ static int cdma_netreg_get_properties(struct modem_data 
*modem)
 static void cm_update_attached(struct modem_data *modem,
DBusMessageIter *value)
 {
-   dbus_message_iter_get_basic(value, &modem->attached);
+   dbus_bool_t attached;
+
+   dbus_message_iter_get_basic(value, &attached);
+   modem->attached = attached;
 
DBG("%s Attached %d", modem->path, modem->attached);
 
-- 
1.8.3.2

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH 0/5] Boolean length issue

[Tomasz Bursztyka]

Here are my findings about boolean length issues.
Patrik has some also to come.

Tomasz Bursztyka (5):
  technology: 32bits boolean should be used due to dbus in relevant
places
  service: 32bits boolean should be used due to dbus in relevant places
  ofono: 32bits boolean should be used due to dbus in relevant places
  vpn: 32bits boolean should be used due to dbus in relevant places
  dundee: 32bits boolean should be used due to dbus in relevant places

 plugins/dundee.c | 10 --
 plugins/ofono.c  |  7 +--
 plugins/vpn.c|  5 -
 src/service.c|  7 +--
 src/technology.c |  4 ++--
 5 files changed, 24 insertions(+), 9 deletions(-)

-- 
1.8.3.2

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH 5/5] dundee: 32bits boolean should be used due to dbus in relevant places

[Tomasz Bursztyka]

---
 plugins/dundee.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/plugins/dundee.c b/plugins/dundee.c
index e49f0c8..b72c69e 100644
--- a/plugins/dundee.c
+++ b/plugins/dundee.c
@@ -539,7 +539,10 @@ static gboolean device_changed(DBusConnection *conn,
 * That means we don't have to order here.
 */
if (g_str_equal(key, "Active")) {
-   dbus_message_iter_get_basic(&value, &info->active);
+   dbus_bool_t active;
+
+   dbus_message_iter_get_basic(&value, &active);
+   info->active = active;
 
DBG("%s Active %d", info->path, info->active);
 
@@ -595,7 +598,10 @@ static void add_device(const char *path, DBusMessageIter 
*properties)
dbus_message_iter_recurse(&entry, &value);
 
if (g_str_equal(key, "Active")) {
-   dbus_message_iter_get_basic(&value, &info->active);
+   dbus_bool_t active;
+
+   dbus_message_iter_get_basic(&value, &active);
+   info->active = active;
 
DBG("%s Active %d", info->path, info->active);
} else if (g_str_equal(key, "Settings")) {
-- 
1.8.3.2

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH 1/5] technology: 32bits boolean should be used due to dbus in relevant places

[Tomasz Bursztyka]

---
 src/technology.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/technology.c b/src/technology.c
index b3adb86..3f11075 100644
--- a/src/technology.c
+++ b/src/technology.c
@@ -839,8 +839,8 @@ static DBusMessage *set_property(DBusConnection *conn,
DBG("property %s", name);
 
if (g_str_equal(name, "Tethering")) {
+   dbus_bool_t tethering;
int err;
-   bool tethering;
 
if (type != DBUS_TYPE_BOOLEAN)
return __connman_error_invalid_arguments(msg);
@@ -913,7 +913,7 @@ static DBusMessage *set_property(DBusConnection *conn,
&technology->tethering_passphrase);
}
} else if (g_str_equal(name, "Powered")) {
-   bool enable;
+   dbus_bool_t enable;
 
if (type != DBUS_TYPE_BOOLEAN)
return __connman_error_invalid_arguments(msg);
-- 
1.8.3.2

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH] service: 32bits boolean should be used due to dbus in relevant places

[Tomasz Bursztyka]

---
 src/service.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/service.c b/src/service.c
index 288deb7..60a9769 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1412,15 +1412,18 @@ static void immutable_changed(struct connman_service 
*service)
 
 static void roaming_changed(struct connman_service *service)
 {
+   dbus_bool_t roaming;
+
if (!service->path)
return;
 
if (!allow_property_changed(service))
return;
 
+   roaming = service->roaming;
connman_dbus_property_changed_basic(service->path,
CONNMAN_SERVICE_INTERFACE, "Roaming",
-   DBUS_TYPE_BOOLEAN, &service->roaming);
+   DBUS_TYPE_BOOLEAN, &roaming);
 }
 
 static void autoconnect_changed(struct connman_service *service)
@@ -3062,7 +3065,7 @@ static DBusMessage *set_property(DBusConnection *conn,
type = dbus_message_iter_get_arg_type(&value);
 
if (g_str_equal(name, "AutoConnect")) {
-   bool autoconnect;
+   dbus_bool_t autoconnect;
 
if (type != DBUS_TYPE_BOOLEAN)
return __connman_error_invalid_arguments(msg);
-- 
1.8.3.2

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH] technology: 32bits boolean should be used due to dbus in relevant places

[Tomasz Bursztyka]

---
 src/technology.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/technology.c b/src/technology.c
index b3adb86..3f11075 100644
--- a/src/technology.c
+++ b/src/technology.c
@@ -839,8 +839,8 @@ static DBusMessage *set_property(DBusConnection *conn,
DBG("property %s", name);
 
if (g_str_equal(name, "Tethering")) {
+   dbus_bool_t tethering;
int err;
-   bool tethering;
 
if (type != DBUS_TYPE_BOOLEAN)
return __connman_error_invalid_arguments(msg);
@@ -913,7 +913,7 @@ static DBusMessage *set_property(DBusConnection *conn,
&technology->tethering_passphrase);
}
} else if (g_str_equal(name, "Powered")) {
-   bool enable;
+   dbus_bool_t enable;
 
if (type != DBUS_TYPE_BOOLEAN)
return __connman_error_invalid_arguments(msg);
-- 
1.8.3.2

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH] wifi: Fix a memory leak when trying to connect a disconnecting network

[Tomasz Bursztyka]

---
 plugins/wifi.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/plugins/wifi.c b/plugins/wifi.c
index 7a80076..bac1fc6 100644
--- a/plugins/wifi.c
+++ b/plugins/wifi.c
@@ -1367,9 +1367,10 @@ static int network_connect(struct connman_network 
*network)
 
ssid_init(ssid, network);
 
-   if (wifi->disconnecting)
+   if (wifi->disconnecting) {
wifi->pending_network = network;
-   else {
+   g_free(ssid);
+   } else {
wifi->network = connman_network_ref(network);
wifi->retries = 0;
 
-- 
1.8.3.2

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH 1/3] settings: Add support for tweaking autoscan parameters

[Tomasz Bursztyka]

In particular cases, it can be useful to tweak such parameters.
Depending on target device.
---
 include/setting.h |  1 +
 src/main.c| 23 ++-
 src/main.conf | 12 
 3 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/include/setting.h b/include/setting.h
index a882021..f492373 100644
--- a/include/setting.h
+++ b/include/setting.h
@@ -31,6 +31,7 @@ extern "C" {
 bool connman_setting_get_bool(const char *key);
 char **connman_setting_get_string_list(const char *key);
 unsigned int *connman_setting_get_uint_list(const char *key);
+char *connman_setting_get_string(const char *key);
 
 unsigned int connman_timeout_input_request(void);
 unsigned int connman_timeout_browser_launch(void);
diff --git a/src/main.c b/src/main.c
index 3a667d6..accfb0f 100644
--- a/src/main.c
+++ b/src/main.c
@@ -41,6 +41,7 @@
 
 #define DEFAULT_INPUT_REQUEST_TIMEOUT (120 * 1000)
 #define DEFAULT_BROWSER_LAUNCH_TIMEOUT (300 * 1000)
+#define DEFAULT_AUTOSCAN_PARAMETERS "exponential:3:300"
 
 #define MAINFILE "main.conf"
 #define CONFIGMAINFILE CONFIGDIR "/" MAINFILE
@@ -72,6 +73,7 @@ static struct {
bool single_tech;
char **tethering_technologies;
bool persistent_tethering_mode;
+   char *autoscan_parameters;
 } connman_settings  = {
.bg_scan = true,
.pref_timeservers = NULL,
@@ -85,6 +87,7 @@ static struct {
.single_tech = false,
.tethering_technologies = NULL,
.persistent_tethering_mode = false,
+   .autoscan_parameters = DEFAULT_AUTOSCAN_PARAMETERS,
 };
 
 #define CONF_BG_SCAN"BackgroundScanning"
@@ -97,8 +100,9 @@ static struct {
 #define CONF_BLACKLISTED_INTERFACES "NetworkInterfaceBlacklist"
 #define CONF_ALLOW_HOSTNAME_UPDATES "AllowHostnameUpdates"
 #define CONF_SINGLE_TECH"SingleConnectedTechnology"
-#define CONF_TETHERING_TECHNOLOGIES  "TetheringTechnologies"
+#define CONF_TETHERING_TECHNOLOGIES "TetheringTechnologies"
 #define CONF_PERSISTENT_TETHERING_MODE  "PersistentTetheringMode"
+#define CONF_AUTOSCAN_PARAMETERS"AutoscanParameters"
 
 static const char *supported_options[] = {
CONF_BG_SCAN,
@@ -113,6 +117,7 @@ static const char *supported_options[] = {
CONF_SINGLE_TECH,
CONF_TETHERING_TECHNOLOGIES,
CONF_PERSISTENT_TETHERING_MODE,
+   CONF_AUTOSCAN_PARAMETERS,
NULL
 };
 
@@ -233,6 +238,7 @@ static void parse_config(GKeyFile *config)
char **interfaces;
char **str_list;
char **tethering;
+   char *parameters;
gsize len;
int timeout;
 
@@ -351,6 +357,13 @@ static void parse_config(GKeyFile *config)
connman_settings.persistent_tethering_mode = boolean;
 
g_clear_error(&error);
+
+   parameters = g_key_file_get_string(config, "General",
+   CONF_AUTOSCAN_PARAMETERS, &error);
+   if (!error)
+   connman_settings.autoscan_parameters = parameters;
+
+   g_clear_error(&error);
 }
 
 static int config_init(const char *file)
@@ -556,6 +569,14 @@ unsigned int *connman_setting_get_uint_list(const char 
*key)
return NULL;
 }
 
+char *connman_setting_get_string(const char *key)
+{
+   if (g_str_equal(key, CONF_AUTOSCAN_PARAMETERS))
+   return connman_settings.autoscan_parameters;
+
+   return NULL;
+}
+
 unsigned int connman_timeout_input_request(void)
 {
return connman_settings.timeout_inputreq;
diff --git a/src/main.conf b/src/main.conf
index b8b3239..cb8d72f 100644
--- a/src/main.conf
+++ b/src/main.conf
@@ -15,11 +15,23 @@
 # BrowserLaunchTimeout = 300
 
 # Enable background scanning. Default is true.
+# This enables 2 different background scanning:
+# - when connected, useful when roaming
+# - when disconnected, useful for autoconnection to known networks
 # Background scanning will start every 5 minutes unless
 # the scan list is empty. In that case, a simple backoff
 # mechanism starting from 10s up to 5 minutes will run.
 # BackgroundScanning = true
 
+# Set the automatic scanning parameters. Default is "exponential:3:300"
+# Used to tweak automatic scanning parameters following this format:
+# :<:param2>
+# when type can be:
+# - exponential: where param1 is the starting delay and param2 the limit.
+# - periodic: where param1 is the delay, there is no param1
+# Unused if BackgroundScanning is false.
+# AutoscanParameters = 
+
 # List of Fallback timeservers separated by ",".
 # These timeservers are used for NTP sync when there are
 # no timeserver set by the user or by the service.
-- 
1.8.3.2

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[pacrunner PATCH] libproxy: Remove useless output

[Tomasz Bursztyka]

Fixes #CM-647
---
 libproxy/proxy.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/libproxy/proxy.c b/libproxy/proxy.c
index e2b4978..7a083eb 100644
--- a/libproxy/proxy.c
+++ b/libproxy/proxy.c
@@ -73,8 +73,6 @@ static char *parse_result(const char *str)
char *result;
int len = 0;
 
-   printf("Parse %s\n", str);
-
if (strcasecmp(str, "DIRECT") == 0)
return strdup("direct://");
 
@@ -140,8 +138,6 @@ static char **extract_results(const char *str)
if (pos == NULL || *pos == '\0' || strlen(pos) < 6)
break;
 
-   printf("pos: %s\n", pos);
-
lookup = pos;
 
c = strchr(pos, ';');
-- 
1.8.2.1

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[pacrunner PATCH v3 4/7] libproxy: Support slicing the result string into relevant list

[Tomasz Bursztyka]

Fixes BMC#26021
---
 libproxy/proxy.c | 126 ++-
 1 file changed, 87 insertions(+), 39 deletions(-)

diff --git a/libproxy/proxy.c b/libproxy/proxy.c
index ae53a7a..e2b4978 100644
--- a/libproxy/proxy.c
+++ b/libproxy/proxy.c
@@ -66,55 +66,100 @@ void px_proxy_factory_free(pxProxyFactory *factory)
free(factory);
 }
 
-static char **extract_result(const char *str)
+static char *parse_result(const char *str)
 {
-   char **result;
+   const char *protocol;
+   int prefix_len;
+   char *result;
+   int len = 0;
 
-   result = malloc(sizeof(char *) * 2);
-   if (result == NULL)
-   return NULL;
-
-   result[0] = NULL;
-   result[1] = NULL;
+   printf("Parse %s\n", str);
 
-   if (strcasecmp(str, "DIRECT") == 0) {
-   result[0] = strdup("direct://");
-   return result;
-   }
+   if (strcasecmp(str, "DIRECT") == 0)
+   return strdup("direct://");
 
if (strncasecmp(str, "PROXY ", 6) == 0) {
-   int len = strlen(str + 6) + 8;
-   result[0] = malloc(len);
-   if (result[0] != NULL)
-   sprintf(result[0], "http://%s";, str + 6);
-   return result;
-   }
+   prefix_len = 6;
+   protocol = "http";
+   len = 8;
+   } else if (strncasecmp(str, "SOCKS ", 6) == 0) {
+   prefix_len = 6;
+   protocol = "socks";
+   len = 9;
+   } else if (strncasecmp(str, "SOCKS4 ", 7) == 0) {
+   prefix_len = 7;
+   protocol = "socks4";
+   len = 10;
+   } else if (strncasecmp(str, "SOCKS5 ", 7) == 0) {
+   prefix_len = 7;
+   protocol = "socks5";
+   len = 10;
+   } else
+   return strdup("direct://");
+
+   len = strlen(str + prefix_len) + len;
+
+   result = malloc(len);
+   if (result != NULL)
+   sprintf(result, "%s://%s", protocol, str + prefix_len);
 
-   if (strncasecmp(str, "SOCKS ", 6) == 0) {
-   int len = strlen(str + 6) + 9;
-   result[0] = malloc(len);
-   if (result[0] != NULL)
-   sprintf(result[0], "socks://%s", str + 6);
-   return result;
-   }
+   return result;
+}
+
+static char **append_result(char **prev_results, int *size, char *result)
+{
+   char **results;
 
-   if (strncasecmp(str, "SOCKS4 ", 7) == 0) {
-   int len = strlen(str + 7) + 10;
-   result[0] = malloc(len);
-   if (result[0] != NULL)
-   sprintf(result[0], "socks4://%s", str + 7);
-   return result;
+   results = realloc(prev_results, sizeof(char *) * (*size + 2));
+   if (results == NULL) {
+   free(result);
+   return prev_results;
}
 
-   if (strncasecmp(str, "SOCKS5 ", 7) == 0) {
-   int len = strlen(str + 7) + 10;
-   result[0] = malloc(len);
-   if (result[0] != NULL)
-   sprintf(result[0], "socks5://%s", str + 7);
-   return result;
+   results[*size] = result;
+   results[*size + 1] = NULL;
+
+   *size = *size + 1;
+
+   return results;
+}
+
+static char **extract_results(const char *str)
+{
+   char *copy_str, *lookup, *pos, *c, *result;
+   char **results = NULL;
+   int nb_results = 0;
+
+   copy_str = strdup(str);
+   if (copy_str == NULL)
+   return NULL;
+
+   pos = copy_str;
+
+   while (1) {
+   if (pos == NULL || *pos == '\0' || strlen(pos) < 6)
+   break;
+
+   printf("pos: %s\n", pos);
+
+   lookup = pos;
+
+   c = strchr(pos, ';');
+   if (c != NULL) {
+   for (*c = '\0', c++;
+   c != NULL && *c == ' '; *c = '\0', c++);
+   }
+
+   pos = c;
+
+   result = parse_result(lookup);
+   if (result != NULL)
+   results = append_result(results, &nb_results, result);
}
 
-   return result;
+   free(copy_str);
+
+   return results;
 }
 
 char **px_proxy_factory_get_proxies(pxProxyFactory *factory, const char *url)
@@ -182,10 +227,13 @@ char **px_proxy_factory_get_proxies(pxProxyFactory 
*factory, const char *url)
if (str == NULL || strlen(str) == 0)
str = "DIRECT";
 
-   result = extract_result(str);
+   result = extract_results(str);
 
dbus_message_unref(reply);
 
+   if (result == NULL)
+   goto direct;
+
return result;
 
 direct:
-- 
1.8.2.1

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: wifi: scan all stored hidden networks relevantly

[Tomasz Bursztyka]

Hi Grant,


On Thu May 16 05:32:29 PDT 2013, Tomasz Bursztyka 
 wrote:
  
-	if (get_hidden_connections(driver_max_ssids, scan_params) > 0) {

+   if (get_hidden_connections_params(wifi, scan_params) > 0) {
ret = g_supplicant_interface_scan(wifi->interface,
scan_params,
-   scan_callback,
+   scan_callback_hidden,

Tomasz,

I found this surprising. Why did the callback change from scan_callback to 
scan_callback_hidden? Isn't the appropriate closure for the scan, scan_callback?



It "loops" on scan_callback_hidden as long as there is hidden ssids to 
scan. We are forced to do such loop due to driver limitation on numbers 
of ssids it can scan at once.


Have a look at get_hidden_connections_params(), it gets all the hidden 
ssid, and use as many as the driver can scan at once. So if there is 20 
ssids and driver can scan only 5: it will scan the 5 firsts, 15 pending, 
then 5 again, 10 pending etc... until 0.


Cheers,

Tomasz


___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH] Tethering: Add the open Wi-Fi Access Point

[Tomasz Bursztyka]

Hi,

NACK from me on this one.

I will pass the discussion whether we want or not such feature.

I will concentrate only on the patch:


--- a/plugins/wifi.c
+++ b/plugins/wifi.c
@@ -2035,6 +2035,7 @@ static int tech_set_tethering(struct connman_technology 
*technology,
}
  
  		connman_technology_tethering_notify(technology, FALSE);

+   connman_technology_reset_tethering_properties(technology);


This is totally bogus: plugins should _never_ override user's settings.
If you want to be able to reset such properties, you will have to let it 
possible through SetProperty() in technology DBus API, nothing else.


  
  		return 0;

}
diff --git a/src/technology.c b/src/technology.c
index f210859..949496d 100644
--- a/src/technology.c
+++ b/src/technology.c
@@ -282,6 +282,20 @@ static void tethering_changed(struct connman_technology 
*technology)
technology_save(technology);
  }
  
+void connman_technology_reset_tethering_properties(struct connman_technology

+  *technology)
+{
+   if (technology->tethering_ident != NULL) {
+   g_free(technology->tethering_ident);
+   technology->tethering_ident = NULL;
+   }
+
+   if (technology->tethering_passphrase != NULL) {
+   g_free(technology->tethering_passphrase);
+   technology->tethering_passphrase = NULL;
+   }
+}
+


This would be useless if you apply my previous comment about SetProperty().


  void connman_technology_tethering_notify(struct connman_technology 
*technology,
connman_bool_t enabled)
  {
@@ -328,8 +342,7 @@ static int set_tethering(struct connman_technology 
*technology,
if (bridge == NULL)
return -EOPNOTSUPP;
  
-	if (technology->type == CONNMAN_SERVICE_TYPE_WIFI &&

-   (ident == NULL || passphrase == NULL))
+   if (technology->type == CONNMAN_SERVICE_TYPE_WIFI && ident == NULL)
return -EINVAL;


Ok.

Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH v2] README: update information about wpa_supplicant built config

[Tomasz Bursztyka]

---

Small typo fixed

 README | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/README b/README
index bfb246a..161c535 100644
--- a/README
+++ b/README
@@ -274,14 +274,21 @@ CONFIG_WPS=y
 CONFIG_AP=y
 CONFIG_CTRL_IFACE_DBUS_NEW=y
 
-and, add:
+add:
 
 CONFIG_BGSCAN_SIMPLE=y
 
 This last option will enable the support of background scanning while being
 connected, which is necessary when roaming on wifi.
 
-It is recommended to use wpa_supplicant 0.8.x or 1.x or later.
+and finally:
+
+CONFIG_AUTOSCAN_EXPONENTIAL=y
+
+This will enable the exact same function as bgscan but while being
+disconnected.
+
+It is recommended to use wpa_supplicant 1.x or later.
 
 
 VPN
-- 
1.8.2.1

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH] README: update information about wpa_supplicant built config

[Tomasz Bursztyka]

---
 README | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/README b/README
index bfb246a..4ad0e83 100644
--- a/README
+++ b/README
@@ -274,14 +274,21 @@ CONFIG_WPS=y
 CONFIG_AP=y
 CONFIG_CTRL_IFACE_DBUS_NEW=y
 
-and, add:
+add:
 
 CONFIG_BGSCAN_SIMPLE=y
 
 This last option will enable the support of background scanning while being
 connected, which is necessary when roaming on wifi.
 
-It is recommended to use wpa_supplicant 0.8.x or 1.x or later.
+and finally:
+
+CONFIG_AUTOSCAN_EXPONENTIAL=y
+
+This will enable the exact same function as bgscan but while beign
+disconnected.
+
+It is recommended to use wpa_supplicant 1.x or later.
 
 
 VPN
-- 
1.8.2.1

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH v2] wifi: scan all stored hidden networks relevantly

[Tomasz Bursztyka]

Currently a scan will only scan the first found stored hidden network.
This patch fixes it: it will scan all, taking into account the limit of
scan parameters the driver can take.

Thanks to Jukka for testing this.
---

1 memory leak and 1 invalid read fixed.
I simplified how frequencies are handled also. 

Fully tested, it seems to work properly.

 gsupplicant/gsupplicant.h |   1 +
 gsupplicant/supplicant.c  |   2 +-
 plugins/wifi.c| 164 +-
 3 files changed, 122 insertions(+), 45 deletions(-)

diff --git a/gsupplicant/gsupplicant.h b/gsupplicant/gsupplicant.h
index 1b1fce2..da45075 100644
--- a/gsupplicant/gsupplicant.h
+++ b/gsupplicant/gsupplicant.h
@@ -148,6 +148,7 @@ struct _GSupplicantScanParams {
 
uint8_t num_ssids;
 
+   uint8_t num_freqs;
uint16_t *freqs;
 };
 
diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c
index 471c0bc..30f0660 100644
--- a/gsupplicant/supplicant.c
+++ b/gsupplicant/supplicant.c
@@ -2742,7 +2742,7 @@ static void add_scan_frequencies(DBusMessageIter *iter,
unsigned int freq;
int i;
 
-   for (i = 0; i < scan_data->num_ssids; i++) {
+   for (i = 0; i < scan_data->num_freqs; i++) {
freq = scan_data->freqs[i];
if (!freq)
break;
diff --git a/plugins/wifi.c b/plugins/wifi.c
index 07180d6..5f07869 100644
--- a/plugins/wifi.c
+++ b/plugins/wifi.c
@@ -107,6 +107,8 @@ struct wifi_data {
 * autoscan "emulation".
 */
struct autoscan_params *autoscan;
+
+   GSupplicantScanParams *scan_params;
 };
 
 static GList *iface_list = NULL;
@@ -263,6 +265,9 @@ static void wifi_remove(struct connman_device *device)
 
g_supplicant_interface_set_data(wifi->interface, NULL);
 
+   if (wifi->scan_params != NULL)
+   g_supplicant_free_scan_params(wifi->scan_params);
+
g_free(wifi->autoscan);
g_free(wifi->identifier);
g_free(wifi);
@@ -290,7 +295,8 @@ static int add_scan_param(gchar *hex_ssid, char *raw_ssid, 
int ssid_len,
unsigned int i;
struct scan_ssid *scan_ssid;
 
-   if (driver_max_scan_ssids > scan_data->num_ssids &&
+   if ((driver_max_scan_ssids == 0 ||
+   driver_max_scan_ssids > scan_data->num_ssids) &&
(hex_ssid != NULL || raw_ssid != NULL)) {
gchar *ssid;
unsigned int j = 0, hex;
@@ -344,36 +350,41 @@ static int add_scan_param(gchar *hex_ssid, char 
*raw_ssid, int ssid_len,
scan_data->ssids = g_slist_reverse(scan_data->ssids);
 
if (scan_data->freqs == NULL) {
-   scan_data->freqs = g_try_malloc0(sizeof(uint16_t) *
-   scan_data->num_ssids);
+   scan_data->freqs = g_try_malloc0(sizeof(uint16_t));
if (scan_data->freqs == NULL) {
g_slist_free_full(scan_data->ssids, g_free);
return -ENOMEM;
}
+
+   scan_data->num_freqs = 1;
+   scan_data->freqs[0] = freq;
} else {
-   scan_data->freqs = g_try_realloc(scan_data->freqs,
-   sizeof(uint16_t) * scan_data->num_ssids);
-   if (scan_data->freqs == NULL) {
-   g_slist_free_full(scan_data->ssids, g_free);
-   return -ENOMEM;
+   connman_bool_t duplicate = FALSE;
+
+   /* Don't add duplicate entries */
+   for (i = 0; i < scan_data->num_freqs; i++) {
+   if (scan_data->freqs[i] == freq) {
+   duplicate = TRUE;
+   break;
+   }
}
-   scan_data->freqs[scan_data->num_ssids - 1] = 0;
-   }
 
-   /* Don't add duplicate entries */
-   for (i = 0; i < scan_data->num_ssids; i++) {
-   if (scan_data->freqs[i] == 0) {
-   scan_data->freqs[i] = freq;
-   break;
-   } else if (scan_data->freqs[i] == freq)
-   break;
+   if (duplicate == FALSE) {
+   scan_data->num_freqs++;
+   scan_data->freqs = g_try_realloc(scan_data->freqs,
+   sizeof(uint16_t) * scan_data->num_freqs);
+   if (scan_data->freqs == NULL) {
+   g_slist_free_full(scan_data->ssids, g_free);
+   return -ENOMEM;
+   }
+   scan_data->freqs[scan_data->num_freqs - 1] = freq;
+   }
}
 
return 1;
 }
 
-static int get_hidden_connections(int max_ssids,
-   GSupplicantScanParams *scan_data)
+static int get_hidden_connections(GSupplicantScanParams *scan_data)
 {
struct connman_conf

[PATCH] wifi: scan all stored hidden networks relevantly

[Tomasz Bursztyka]

Currently a scan will only scan the first found stored hidden network.
This patch fixes it: it will scan all, taking into account the limit of
scan parameters the driver can take.
---

Tested it quickly and seems to work, while enforcing driver_max_ssids to be 1.

Please review,

 plugins/wifi.c | 117 -
 1 file changed, 99 insertions(+), 18 deletions(-)

diff --git a/plugins/wifi.c b/plugins/wifi.c
index 07180d6..8898d75 100644
--- a/plugins/wifi.c
+++ b/plugins/wifi.c
@@ -64,6 +64,8 @@
 #define BGSCAN_DEFAULT "simple:30:-45:300"
 #define AUTOSCAN_DEFAULT "exponential:3:300"
 
+#define MAX_HIDDEN_SSIDS_STORAGE 256
+
 static struct connman_technology *wifi_technology = NULL;
 
 struct hidden_params {
@@ -107,6 +109,8 @@ struct wifi_data {
 * autoscan "emulation".
 */
struct autoscan_params *autoscan;
+
+   GSupplicantScanParams *scan_params;
 };
 
 static GList *iface_list = NULL;
@@ -263,6 +267,9 @@ static void wifi_remove(struct connman_device *device)
 
g_supplicant_interface_set_data(wifi->interface, NULL);
 
+   if (wifi->scan_params != NULL)
+   g_supplicant_free_scan_params(wifi->scan_params);
+
g_free(wifi->autoscan);
g_free(wifi->identifier);
g_free(wifi);
@@ -466,6 +473,84 @@ static int get_hidden_connections(int max_ssids,
return num_ssids > max_ssids ? max_ssids : num_ssids;
 }
 
+static int get_hidden_connections_params(struct wifi_data *wifi,
+   GSupplicantScanParams *scan_params)
+{
+   int driver_max_ssids, i;
+   GSupplicantScanParams *orig_params;
+
+   /*
+* Scan hidden networks so that we can autoconnect to them.
+* We will assume 1 as a default number of ssid to scan.
+*/
+   driver_max_ssids = g_supplicant_interface_get_max_scan_ssids(
+   wifi->interface);
+   if (driver_max_ssids == 0)
+   driver_max_ssids = 1;
+
+   DBG("max ssids %d", driver_max_ssids);
+
+   if (wifi->scan_params == NULL) {
+   wifi->scan_params = 
g_try_malloc0(sizeof(GSupplicantScanParams));
+   if (wifi->scan_params == NULL)
+   return 0;
+
+   if (get_hidden_connections(MAX_HIDDEN_SSIDS_STORAGE,
+   wifi->scan_params) == 0) {
+   g_supplicant_free_scan_params(wifi->scan_params);
+   wifi->scan_params = NULL;
+
+   return 0;
+   }
+   }
+
+   orig_params = wifi->scan_params;
+
+   /* Let's transfer driver_max_ssids params */
+   for (i = 0; i < driver_max_ssids; i++) {
+   struct scan_ssid *ssid;
+
+   if (wifi->scan_params->ssids == NULL)
+   break;
+
+   ssid = orig_params->ssids->data;
+   orig_params->ssids = g_slist_remove(orig_params->ssids, ssid);
+   scan_params->ssids = g_slist_prepend(scan_params->ssids, ssid);
+   }
+
+   if (i > 0) {
+   uint16_t *n_freqs;
+
+   scan_params->num_ssids = i;
+   scan_params->ssids = g_slist_reverse(scan_params->ssids);
+
+   scan_params->freqs = g_memdup(orig_params->freqs,
+   sizeof(uint16_t) * i);
+   if (scan_params->freqs == NULL)
+   goto err;
+
+   n_freqs = g_memdup(orig_params->freqs+i,
+   sizeof(uint16_t) * orig_params->num_ssids - i);
+   if (n_freqs == NULL)
+   goto err;
+
+   g_free(orig_params->freqs);
+   orig_params->freqs = n_freqs;
+   } else
+   goto err;
+
+   orig_params->num_ssids -= scan_params->num_ssids;
+
+   return scan_params->num_ssids;
+
+err:
+   g_slist_free_full(scan_params->ssids, g_free);
+   g_supplicant_free_scan_params(wifi->scan_params);
+   wifi->scan_params = NULL;
+
+   return 0;
+}
+
 static int throw_wifi_scan(struct connman_device *device,
GSupplicantInterfaceCallback callback)
 {
@@ -514,10 +599,17 @@ static void scan_callback(int result, 
GSupplicantInterface *interface,
 
DBG("result %d wifi %p", result, wifi);
 
-   if (wifi != NULL && wifi->hidden != NULL) {
-   connman_network_clear_hidden(wifi->hidden->user_data);
-   hidden_free(wifi->hidden);
-   wifi->hidden = NULL;
+   if (wifi != NULL) {
+   if (wifi->hidden != NULL) {
+   connman_network_clear_hidden(wifi->hidden->user_data);
+   hidden_free(wifi->hidden);
+   wifi->hidden = NULL;
+   }
+
+   if (wifi->scan_params != NULL) {
+   g_supplicant_free_scan_params

Re: API proposal for WiFi Direct support

[Tomasz Bursztyka]

Hi Simon,

I am not completely aware of P2P stuff, so some of my comments might be 
bogus.



Attached to this mail are three
patches which outline the needed modifications to support group formation (with 
GO
negotiation and autonoumous group support). It would be great if you can take a 
look at
the patches and tell me what you think about the proposal. I tried to not break 
the API in
any part and to build upon existing things.


Technolgy and Service part looks ok to me.
The group thing looks complicated: a lot to do in user side.
Wouldn't it be possible simplify it? (default group, etc...)
It might even be wise to solve the simplest use case of p2p, so without 
such group interface.


Btw, in case a connman device get's invited to connect: how would it be 
handled? It probably requires agent stuff here.



One thing which is missing is the service discovery functional. I am not sure 
wether this
is something which belongs into connman or should be better provided by some 
other
component.


Underlying wifi daemon (wpa_supplicant) should handle that. I don't know 
how much wpa_supplicant supports about P2P though.



Br,

Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH 1/3] doc: add documentation for P2P related API modifications

[Tomasz Bursztyka]

Hi Simon,

+   boolean P2P [readwrite]
+
+   This option allows to enable or disable the support
+   for P2P wireless network also known as WiFi Direct.
+
+   When P2P is enabled scanning will scan for P2P peer
+   clients as well.


Ok


+
+   string P2PIdentifier [readwrite]
+
+   This option allows to set the name of the device
+   used in P2P communication.


Why not using system's hostname directly? At least as a default value.
I remember we had the same discussion about wifi tethering SSID.
Btw, you might add that this is available only for wifi technology.

Br,

Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: PowerPC Build Failure

[Tomasz Bursztyka]

Hi Jack,



However on powerpc64 bits/ioctl-types.h doesn't include asm/ioctls.h 
so asm-generic/ioctls.h never gets included.


Therefore TCGETS2 never gets defined. Is this a bug somewhere further 
upstream, or does it just mean that powerpc64 doesn't support the 
asm-generic ioctls? 


I don't know much about ppc64, but without such inclusion it can't do 
much around ttys. I would bet on an issue on your OE configuration somehow.



Br,

Tomasz

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: PowerPC Build Failure

[Tomasz Bursztyka]

Hi Jack,


| plugins/tist.c: In function 'set_custom_baud_rate':
| plugins/tist.c:283:18: error: 'TCGETS2' undeclared (first use in 
this function)
| plugins/tist.c:283:18: note: each undeclared identifier is reported 
only once for each function it appears in
| plugins/tist.c:291:18: error: 'TCSETS2' undeclared (first use in 
this function)


Does you system supports termios2?

Br,

Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: gadget ethernet problem

[Tomasz Bursztyka]

Hi Petr,

Your logs go fine until src/device.c:connman_device_register() then... 
no __connman_technology_add_device()


Could you rerun connman but with: connmand -n -d 
src/device.c,src/technology.c,src/rtnl.c,plugins/ethernet.c

Just modprobe your usb module, don't configure anything with ifconfig.


Tomasz

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: gadget ethernet problem

[Tomasz Bursztyka]

Hi Petr,

Just noticed that you are using connman 1.8 which is not that up to 
date. Could you switch to later version?


From your log it looks like the technology is properly registered, and 
device found etc... maybe a bug somewhere.


Tomasz
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: gadget ethernet problem

[Tomasz Bursztyka]

Hi Petr,

Afaik you should see it as "gadget" in technology list. Do you see it? 
Is it enabled?


connmand[268]: usb0 {newlink} index 3 operstate 2  


Dip you plug the ethernet cable?

Tomasz
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: [PACRunner PATCH 0/2] Fix JAVASCRIPT_ROUTINES for v8

[Tomasz Bursztyka]

Hi Joshua,

Thanks for the patch!

ACK from me.

Tomasz


We'd noticed that PACRunner's FindProxyForURL() was always returning DIRECT
when using the v8 backend. Tomasz helped trace the issue to an error from v8

"Uncaught TypeError: object is not a function"

Which, with some further digging, was revealed to be some non-standard
JavaScript in isInNet().

With the following minor patches I'm able to use v8 with PACRunner.

Cheers,

Joshua

Joshua Lock (2):
   Fix linking of unit tests
   Fix isInNet() to work with non-Mozilla JS interpreters

  Makefile.am  | 2 +-
  src/javascript.h | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)



___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: Wifi direct support for connman

[Tomasz Bursztyka]

Hi Simon,

Afaik nobody is working on it. Some people before have told having the 
same interest as you on wifi direct (or p2p) but  no news since then.

So patches are warmly welcome!

Br,

Tomasz


Hey everybody,

I am pretty interested in wifi direct support within connman. I know 
there were some efforts to get a proof of concept implemented.


Before I start over working on this I would like to ask wether anybody 
else is working on wifi direct support for connman so we can share our 
efforts instead of doing the same work twice.


regards,
Simon



___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH] neard: RequestOOB() method sends empty dictionnary

[Tomasz Bursztyka]

Reported by Ravikumar Veeramally
---
 plugins/neard.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/plugins/neard.c b/plugins/neard.c
index 85cf06b..83561fe 100644
--- a/plugins/neard.c
+++ b/plugins/neard.c
@@ -172,6 +172,9 @@ static int parse_request_oob_params(DBusMessage *message,
const char *key;
int arg_type;
 
+   if (tlv_msg == NULL || length == NULL)
+   return -EINVAL;
+
dbus_message_iter_init(message, &iter);
 
if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_ARRAY)
@@ -182,9 +185,6 @@ static int parse_request_oob_params(DBusMessage *message,
if (arg_type != DBUS_TYPE_DICT_ENTRY)
return -EINVAL;
 
-   if (tlv_msg == NULL && length == NULL)
-   return 0;
-
while (arg_type != DBUS_TYPE_INVALID) {
dbus_message_iter_recurse(&array, &dict_entry);
if (dbus_message_iter_get_arg_type(&dict_entry) !=
@@ -254,9 +254,13 @@ out:
 static DBusMessage *request_oob_method(DBusConnection *dbus_conn,
DBusMessage *message, void *user_data)
 {
+   DBusMessageIter iter;
+
DBG("");
 
-   if (parse_request_oob_params(message, NULL, NULL) != 0)
+   dbus_message_iter_init(message, &iter);
+
+   if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_ARRAY)
return get_reply_on_error(message, EINVAL);
 
return create_request_oob_reply(message);
-- 
1.8.1.5

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: [PATCH v6 00/14] iptables refactoring

[Tomasz Bursztyka]

Hi Daniel,

ACK from me :)

Tomasz
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: [RFC] iptables-test: Add dump command

[Tomasz Bursztyka]

Hi Daniel,


Hi Tomasz,

What do you think about this? Yes I agree the __connman_log_init()
is dirty but we can't really add '-d' because that belongs also
to iptables command set. And enabling __connman_log_init() always
is bit too much in my opinion.


Looks fine to me, iptables-test is a test tool anyway.

I just tried, sure the output is a bit cryptic and verbose compared to 
real iptables but I think it's fine.

It should help actually to figure out some bugs, if any, in src/iptables.c
And anyway, when testing new rules addition/deletion, dev should always 
check via the real iptables if everything is correct.


Just 2 comments below:


diff --git a/tools/iptables-test.c b/tools/iptables-test.c
index 95d0af4..bef9251 100644
--- a/tools/iptables-test.c
+++ b/tools/iptables-test.c
@@ -34,6 +34,7 @@ enum iptables_command {
IPTABLES_COMMAND_POLICY,
IPTABLES_COMMAND_CHAIN_INSERT,
IPTABLES_COMMAND_CHAIN_DELETE,
+   IPTABLES_COMMAND_DUMP,
IPTABLES_COMMAND_UNKNOWN,
  };
  
@@ -42,11 +43,12 @@ int main(int argc, char *argv[])

enum iptables_command cmd = IPTABLES_COMMAND_UNKNOWN;
char *table = NULL, *chain = NULL, *rule = NULL, *tmp;


Fix table so it gets "filter" as a default value. If you run 
./iptables-test  without -t it should imply that "fitler" table is used. 
(like with real iptables)

Currently, not using -t makes it segfault on commit.


int err, c, i;
+   char *debug;


Unused so you can nuke it.

Merge this patch + my comments to your patch 4 and you get my ACK.

Cheers,

Tomasz
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: [PATCH v5 00/12] [RESEND] iptables refactoring

[Tomasz Bursztyka]

Hi Daniel,

ACK from me. Let's put aside patch 4 as we discussed.

Cheers,

Tomasz
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: Connman upstream test result_20130308

[Tomasz Bursztyka]

Hi,

reproduced bugs:
===
25125 - There is still IP address on client after disable-tethering bluetooth
https://bugs.meego.com/show_bug.cgi?id=25125



This one is no longer relevant to reproduce with wrong version of BlueZ. 
It has been resolved/fixed for a long time already.

So let's forget about it.

Br,

Tomasz
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: [PATCH v4 10/11] iptables: Refactor iptables_delete_rule()

[Tomasz Bursztyka]

Hi Daniel,

And then I just realized that in iptables_flush_chain() the exact open 
coded update function exists: 


Of course duplication is not ok.
See, it was worth being picky: now you found a interesting 
refactorization point, which you patch was not correcting.


Please redo your patch with that change. ;)

Cheers,

Tomasz
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: [PATCH v4 00/11] iptables refactroring

[Tomasz Bursztyka]

Hi Daniel,


I really should spend more time in explaining stuff in the cover letter.

These patches are kind of left overs from the bug fixing session and 
not really necessary if you think in 'does it fix a bug' or 'it's a 
new feature'. These patches are just plain maintaining work which 
improve the structure of the code and should help to the next reader 
not to spend weeks going over the code again and again which I did.




It's like that in many parts in connman, and the code has not much to do 
here: usually it's because the domain it's touching is really complex.
And the coder will have to make effort to understand the domain to 
understand the code.
In iptables.c the complexity comes from xtables, any rewrite won't fix 
it. And you know that.
Talking about next reader in this specific context is not really an 
argument.


In most of your rewrite in later patches, the comments your added are 
the real worthy part, not the code rewrite itself.


I understand you spend much time on iptables.c to understand it, I went 
through the same hassle. But refactoring is a trap, especially there.

Moreover it can introduce regressions.

I agree with patches 1,2,3,4,5,6,7. The rest it too much change for - 
objectively - no real benefit. Besides the comments.


Cheers,

Tomasz

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: [PATCH v4 10/11] iptables: Refactor iptables_delete_rule()

[Tomasz Bursztyka]

Le 07/03/2013 15:35, Daniel Wagner a écrit :

-   /* We have deleted a rule,
-* all references should be bumped accordingly */
+   /*
+* We have deleted a rule, all references should be bumped
+* accordingly
+*/


What an improvement ;]
Just kidding.

Seriously, here again I don't see it clarifies the code more.
Actually, only the comments you added do clarify things, these are 
relevant yes!


Tomasz
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: [PATCH v4 11/11] iptables: find_chain_tail should return last entry

[Tomasz Bursztyka]

Le 07/03/2013 16:45, Daniel Wagner a écrit :

On 03/07/2013 03:33 PM, Patrik Flykt wrote:

On Thu, 2013-03-07 at 14:35 +0100, Daniel Wagner wrote:

Currently, find_chain_tail() returns the element after the chain end.


Let's keep it the old way until there is a real reason to do something
else. If it ain't broken, let's not fix it (yet).


This function is broken. So I fix it. And I disagree with you reasoning.



I don't see a fix here, and afaik this function would be broken not much 
code from iptables.c would work, wouldn't it? ;)
You are just changing the signature of find_chain_tail() and if you want 
to do so, you should do the same for find_chain_head()


But from semantic point of view: tail and head are words tighten to 
list. tail of the list, head of the list. Here you change the stuff then.


And personally I don't see the code much readable before or after.

Tomasz
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: [PATCH v3 05/10] iptables: Improve debug log output

[Tomasz Bursztyka]

Hi Daniel,


@@ -1669,7 +1677,18 @@ static struct connman_iptables *pre_load_table(const 
char *table_name,
if (table != NULL)
return table;
  
-	return iptables_init(table_name);

+   table = g_hash_table_lookup(table_hash, table_name);
+   if (table != NULL)
+   return table;
+
+   table = iptables_init(table_name);
+   if (table == NULL)
+   return NULL;
+
+   table->name = g_strdup(table_name);
+   g_hash_table_replace(table_hash, table->name, table);
+


This part has nothing to do with debug improvement. Split this up.

Tomasz
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: [PATCH v5 0/7] iptables bug fixes

[Tomasz Bursztyka]

ACK from me!


From: Daniel Wagner 

changes:
- patch #6: use opencoding for verdict
- patch #7: fix append/insert confusion


cheers,
daniel

original cover letter

This is a subset which contains only bugfixes from the series called
'[PATCH v0 00/16] Managed iptables API'.

Daniel Wagner (7):
   iptables: Fix is_fallthrough() check
   iptables: Fix and refactor iterate_entries()
   iptables: Do not flush in the wrong order
   iptables: Always update options table
   iptables: Fix setting policy
   iptables: Valid policies are only ACCEPT and DROP
   iptables: Fix rule appending

  src/iptables.c | 120 +
  1 file changed, 78 insertions(+), 42 deletions(-)



___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: [PATCH v3 7/9] iptables: Remove wrong sanity check in prepare_rule_inclusion()

[Tomasz Bursztyka]

Hi Daniel,



The definition is pretty clear in my opinion. 'append' means insert 
rule at the end of the chain, 'insert' at any position defined by the 
index. no?




No index in the story here. Moreover we don't handle rule numbers in our 
code. append put the rule at the and of the list, insert puts the rules 
at the beginning of the list.


See my latest answer of patch 9/9. append used to append.

Tomasz
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: [PATCH v3 9/9] iptables: Fix setting policy

[Tomasz Bursztyka]

Le 06/03/2013 16:32, Daniel Wagner a écrit :

Hi Tomasz,

On 03/06/2013 02:11 PM, Tomasz Bursztyka wrote:

The orignal code assumes that the builtin chain is empty which is
obviously always the right assumption :).


That is actually the bug in the original code: it might not be empty. If
it is, it does the right change, if not it modifies the 1 rule in the
chain.


I am looking at the current behaviour of __connman_iptables_append() 
and I think it is broken. No patches applied nor have I have touched 
this code in previous patches.


No shit sherlock, that's what I have told in response to your patch 7/9 ;-P

Look at iptables.c:
__connman_iptables_append() and __connman_iptables_insert()

Now go back to 1.11 before YOUR previous patchset:

look at iptables_append_rule() and iptables_insert_rule()

Tomasz
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman