2017-09-01 03:56:15

by Yunhan Wang

[permalink] [raw]
Subject: [PATCH BlueZ] client: Fix default_ctrl change when new adapter is found

When another adapter is found, the default adapter would be changed,
which is not expected. Default adapter can only be changed by select
command.
---
client/main.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/client/main.c b/client/main.c
index 75696c2c1..29b3978e1 100644
--- a/client/main.c
+++ b/client/main.c
@@ -67,6 +67,7 @@ struct adapter {
};

static struct adapter *default_ctrl;
+static struct adapter *cache_ctrl;
static GDBusProxy *default_dev;
static GDBusProxy *default_attr;
static GList *ctrl_list;
@@ -151,7 +152,7 @@ static void disconnect_handler(DBusConnection *connection, void *user_data)

g_list_free_full(ctrl_list, proxy_leak);
ctrl_list = NULL;
-
+ cache_ctrl = NULL;
default_ctrl = NULL;
}

@@ -521,15 +522,21 @@ static void device_added(GDBusProxy *proxy)

static void adapter_added(GDBusProxy *proxy)
{
- default_ctrl = g_malloc0(sizeof(struct adapter));
- default_ctrl->proxy = proxy;
- ctrl_list = g_list_append(ctrl_list, default_ctrl);
+ struct adapter *adapter = g_malloc0(sizeof(struct adapter));
+
+ adapter->proxy = proxy;
+ cache_ctrl = adapter;
+ ctrl_list = g_list_append(ctrl_list, adapter);
+
+ if (!default_ctrl)
+ default_ctrl = adapter;
+
print_adapter(proxy, COLORED_NEW);
}

static void ad_manager_added(GDBusProxy *proxy)
{
- default_ctrl->ad_proxy = proxy;
+ cache_ctrl->ad_proxy = proxy;
}

static void proxy_added(GDBusProxy *proxy, void *user_data)
@@ -602,7 +609,6 @@ static void adapter_removed(GDBusProxy *proxy)
print_adapter(proxy, COLORED_DEL);

if (default_ctrl && default_ctrl->proxy == proxy) {
- default_ctrl = NULL;
set_default_device(NULL, NULL);
}

--
2.14.1.581.gf28d330327-goog



2017-09-06 17:12:46

by Zhang, Ming

[permalink] [raw]
Subject: RE: [PATCH BlueZ] client: Fix default_ctrl change when new adapter is found

VGhhbmtzIFl1bmhhbiwgDQoNCldlIGhhdmUgZmlndXJlZCBvdXQgaG93IHRvIHVzZSB0aG9zZSBj
b21tYW5kcyBpbiBibHVldG9vdGhjdGwuIEFsc28gd2UgYnVpbHQgMiBoaWdoIGxldmVsIHRvb2xz
LCBnYXR0LWNsaWVudCBhbmQgZ2F0dC1zZXJ2ZXIsIHdpdGggdGhlc2UgdG9vbHMsIHJlYWQvd3Jp
dGUgb3BlcmF0aW9ucyBhcmUgbW9yZSBjb252ZW5pZW50LiANCg0KUmVnYXJkcywgDQpNaW5nDQoN
Ci0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQpGcm9tOiBZdW5oYW4gV2FuZyBbbWFpbHRvOnl1
bmhhbndAZ29vZ2xlLmNvbV0gDQpTZW50OiBUdWVzZGF5LCBTZXB0ZW1iZXIgMDUsIDIwMTcgMTA6
NDMgUE0NClRvOiBaaGFuZywgTWluZyA8TWluZy5aaGFuZ0BhcnJpcy5jb20+DQpDYzogRVJBTU9U
TyBNYXNheWEgPGVyYW1vdG8ubWFzYXlhQGpwLmZ1aml0c3UuY29tPjsgbGludXgtYmx1ZXRvb3Ro
QHZnZXIua2VybmVsLm9yZw0KU3ViamVjdDogUmU6IFtQQVRDSCBCbHVlWl0gY2xpZW50OiBGaXgg
ZGVmYXVsdF9jdHJsIGNoYW5nZSB3aGVuIG5ldyBhZGFwdGVyIGlzIGZvdW5kDQoNCkhpLCBNaW5n
DQoNCkNvdWxkIHlvdSBwb3N0IHlvdXIgY29tbWFuZCBmcm9tIHlvdXIgcGVyaXBoZXJhbCBhbmQg
Y2VudHJhbD8gVXN1YWxseSB3aGVuIHlvdSBzZXQgdGhlIGF0dHJpYnV0ZSB3aXRoIHJlYWQsd3Jp
dGUgcGVybWlzc2lvbiBpbiBwZXJpcGhlcmFsLCB0aGVuIGluIGNlbnRyYWwsIHlvdSBjYW4gc2Vs
ZWN0LWF0dHJpYnV0ZSB0aGUgY2hhcmFjdGVyaXN0aWMgcGF0aCwgYW5kIHJlYWQvd3JpdGUuDQoN
CnRoYW5rcw0KYmVzdCB3aXNoZXMNCnl1bmhhbg0KDQpPbiBGcmksIFNlcCAxLCAyMDE3IGF0IDI6
MDYgUE0sIFpoYW5nLCBNaW5nIDxNaW5nLlpoYW5nQGFycmlzLmNvbT4gd3JvdGU6DQo+IEhlbGxv
LA0KPg0KPiBDYW4gYW55Ym9keSBnaXZlIGEgaGVscCB3aXRoIEJsdWVaIHV0aWxpdHkgYmx1ZXRv
b3RoY3RsPw0KPiAtICJyZWFkIiBhbmQgIndyaXRlIiBjb21tYW5kcyBzZWVtIG5vdCB3b3JraW5n
PyBhbHdheXMgcmVwb3J0ICJubyBhdHRyaWJ1dGUgc2VsZWN0ZWQiLCB1c2luZyAic2VsZWN0LWF0
dHJpYnV0ZSIgZmlyc3QgZG9lc24ndCBoZWxwLg0KPg0KPiBUaGFua3MsDQo+IE1pbmcNCj4NCj4g
LS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogbGludXgtYmx1ZXRvb3RoLW93bmVy
QHZnZXIua2VybmVsLm9yZyANCj4gW21haWx0bzpsaW51eC1ibHVldG9vdGgtb3duZXJAdmdlci5r
ZXJuZWwub3JnXSBPbiBCZWhhbGYgT2YgWXVuaGFuIA0KPiBXYW5nDQo+IFNlbnQ6IEZyaWRheSwg
U2VwdGVtYmVyIDAxLCAyMDE3IDEyOjQzIEFNDQo+IFRvOiBFUkFNT1RPIE1hc2F5YSA8ZXJhbW90
by5tYXNheWFAanAuZnVqaXRzdS5jb20+DQo+IENjOiBsaW51eC1ibHVldG9vdGhAdmdlci5rZXJu
ZWwub3JnDQo+IFN1YmplY3Q6IFJlOiBbUEFUQ0ggQmx1ZVpdIGNsaWVudDogRml4IGRlZmF1bHRf
Y3RybCBjaGFuZ2Ugd2hlbiBuZXcgDQo+IGFkYXB0ZXIgaXMgZm91bmQNCj4NCj4gSGksIEVSQU1P
VE8NCj4NCj4gY2FjaGVfY3RybCBpcyBub3QgYWR2ZXJ0aXNpbmcgbWFuYWdlciwgY2FjaGVfY3Ry
bCBpcyB0ZW1wb3JhcnkgYWRhcHRlciB0aGF0IHN0b3JlIG5ld2VzdCBhZGFwdGVyIGluZm8oc2Vl
IHRoZSBiZWxvdyBzdHJ1Y3R1cmUgaW5mbykuIFdoZW4geW91IHVzZSBzZWxlY3QsIGZpbmRfY3Ry
bF9ieV9hZGRyZXNzIHdvdWxkIGZpbHRlciB5b3VyIHByZWZlcnJlZCBhZGFwdGVyIGJ5IHlvdXIg
YWRkcmVzcywgdGhlbiBpZiBpdCBpcyBkZWZhdWx0LCByZXR1cm4sIG90aGVyd2lzZSwgaXQgd291
bGQgdXBkYXRlIGRlZmF1bHRfY3RybC4NCj4NCj4gc3RydWN0IGFkYXB0ZXIgew0KPiBHREJ1c1By
b3h5ICpwcm94eTsNCj4gR0RCdXNQcm94eSAqYWRfcHJveHk7DQo+IEdMaXN0ICpkZXZpY2VzOw0K
PiB9Ow0KPg0KPiB3aGVuIG5ldyBhZGFwdGVyIGlzIGZvdW5kLCB0aGUgcHJveHkgaGFuZGxlciB3
b3VsZCBhZGQgYWRhcHRlciBwcm94eSwgcHJvZmlsZSBwcm94eSwgYWR2ZXJ0aXNpbmcgcHJveHkg
c2VxdWVudGlhbGx5LCBpbiB0aGlzIHByb2NlZHVyZSwgY2FjaGVfY3RybCB3b3VsZCB1cGRhdGUg
YWR2ZXJ0aXNpbmcgbWFuYWdlci4NCj4NCj4gdGhhbmtzDQo+IGJlc3Qgd2lzaGVzDQo+IHl1bmhh
bg0KPg0KPiBPbiBGcmksIFNlcCAxLCAyMDE3IGF0IDEyOjI4IEFNLCBFUkFNT1RPIE1hc2F5YSA8
ZXJhbW90by5tYXNheWFAanAuZnVqaXRzdS5jb20+IHdyb3RlOg0KPj4gSGkgWXVuaGFuLA0KPj4N
Cj4+IFlvdXIgcGF0Y2ggaGF2ZSBhIHJlZ3Jlc3Npb24sIHNpbmNlIHNlbGVjdCBjb21tYW5kIGNh
biBub3QgdXBkYXRlIHRoZSANCj4+IGN1cnJlbnQgYWR2ZXJ0aXNpbmcgbWFuYWdlciAoY2FjaGVf
Y3RybCkgd2hlbiBpdCBjaG9vc2VzIGFub3RoZXIgYWRhcHRlci4NCj4+DQo+PiBJIHRoaW5rIHRo
YXQgaXQgaXMgYmV0dGVyIHRvIHVzZSBwcm94eS0+b2JqX3BhdGggYmVsb3c6DQo+Pg0KPj4gLS0t
DQo+PiAgY2xpZW50L21haW4uYyB8IDU3DQo+PiArKysrKysrKysrKysrKysrKysrKysrKysrKysr
KysrKysrKysrKystLS0tLS0tLS0tLS0tLS0tLS0NCj4+ICAxIGZpbGUgY2hhbmdlZCwgMzkgaW5z
ZXJ0aW9ucygrKSwgMTggZGVsZXRpb25zKC0pDQo+Pg0KPj4gZGlmZiAtLWdpdCBhL2NsaWVudC9t
YWluLmMgYi9jbGllbnQvbWFpbi5jIGluZGV4IDgyNTY0N2QuLmM2Y2I5OTUNCj4+IDEwMDY0NA0K
Pj4gLS0tIGEvY2xpZW50L21haW4uYw0KPj4gKysrIGIvY2xpZW50L21haW4uYw0KPj4gQEAgLTUy
NSwxNyArNTI1LDUyIEBAIHN0YXRpYyB2b2lkIGRldmljZV9hZGRlZChHREJ1c1Byb3h5ICpwcm94
eSkNCj4+ICAgICAgICAgfQ0KPj4gIH0NCj4+DQo+PiArc3RhdGljIHN0cnVjdCBhZGFwdGVyICpm
aW5kX2N0cmwoR0xpc3QgKnNvdXJjZSwgY29uc3QgY2hhciAqcGF0aCkgew0KPj4gKyAgICAgICBH
TGlzdCAqbGlzdDsNCj4+ICsNCj4+ICsgICAgICAgZm9yIChsaXN0ID0gZ19saXN0X2ZpcnN0KHNv
dXJjZSk7IGxpc3Q7IGxpc3QgPSBnX2xpc3RfbmV4dChsaXN0KSkgew0KPj4gKyAgICAgICAgICAg
ICAgIHN0cnVjdCBhZGFwdGVyICphZGFwdGVyID0gbGlzdC0+ZGF0YTsNCj4+ICsNCj4+ICsgICAg
ICAgICAgICAgICBpZiAoIXN0cmNhc2VjbXAoZ19kYnVzX3Byb3h5X2dldF9wYXRoKGFkYXB0ZXIt
PnByb3h5KSwgcGF0aCkpDQo+PiArICAgICAgICAgICAgICAgICAgICAgICByZXR1cm4gYWRhcHRl
cjsNCj4+ICsgICAgICAgfQ0KPj4gKw0KPj4gKyAgICAgICByZXR1cm4gTlVMTDsNCj4+ICt9DQo+
PiArDQo+PiArc3RhdGljIHN0cnVjdCBhZGFwdGVyICphZGFwdGVyX25ldyhHREJ1c1Byb3h5ICpw
cm94eSkgew0KPj4gKyAgICAgICBzdHJ1Y3QgYWRhcHRlciAqYWRhcHRlciA9IGdfbWFsbG9jMChz
aXplb2Yoc3RydWN0IGFkYXB0ZXIpKTsNCj4+ICsNCj4+ICsgICAgICAgY3RybF9saXN0ID0gZ19s
aXN0X2FwcGVuZChjdHJsX2xpc3QsIGFkYXB0ZXIpOw0KPj4gKw0KPj4gKyAgICAgICBpZiAoIWRl
ZmF1bHRfY3RybCkNCj4+ICsgICAgICAgICAgICAgICBkZWZhdWx0X2N0cmwgPSBhZGFwdGVyOw0K
Pj4gKw0KPj4gKyAgICAgICByZXR1cm4gYWRhcHRlcjsNCj4+ICt9DQo+PiArDQo+PiAgc3RhdGlj
IHZvaWQgYWRhcHRlcl9hZGRlZChHREJ1c1Byb3h5ICpwcm94eSkgIHsNCj4+IC0gICAgICAgZGVm
YXVsdF9jdHJsID0gZ19tYWxsb2MwKHNpemVvZihzdHJ1Y3QgYWRhcHRlcikpOw0KPj4gLSAgICAg
ICBkZWZhdWx0X2N0cmwtPnByb3h5ID0gcHJveHk7DQo+PiAtICAgICAgIGN0cmxfbGlzdCA9IGdf
bGlzdF9hcHBlbmQoY3RybF9saXN0LCBkZWZhdWx0X2N0cmwpOw0KPj4gKyAgICAgICBzdHJ1Y3Qg
YWRhcHRlciAqY3RybDsNCj4+ICsgICAgICAgY3RybCA9IGZpbmRfY3RybChjdHJsX2xpc3QsIGdf
ZGJ1c19wcm94eV9nZXRfcGF0aChwcm94eSkpOw0KPj4gKyAgICAgICBpZiAoIWN0cmwpDQo+PiAr
ICAgICAgICAgICAgICAgY3RybCA9IGFkYXB0ZXJfbmV3KHByb3h5KTsNCj4+ICsNCj4+ICsgICAg
ICAgY3RybC0+cHJveHkgPSBwcm94eTsNCj4+ICsNCj4+ICAgICAgICAgcHJpbnRfYWRhcHRlcihw
cm94eSwgQ09MT1JFRF9ORVcpOyAgfQ0KPj4NCj4+ICBzdGF0aWMgdm9pZCBhZF9tYW5hZ2VyX2Fk
ZGVkKEdEQnVzUHJveHkgKnByb3h5KSAgew0KPj4gLSAgICAgICBkZWZhdWx0X2N0cmwtPmFkX3By
b3h5ID0gcHJveHk7DQo+PiArICAgICAgIHN0cnVjdCBhZGFwdGVyICpjdHJsOw0KPj4gKyAgICAg
ICBjdHJsID0gZmluZF9jdHJsKGN0cmxfbGlzdCwgZ19kYnVzX3Byb3h5X2dldF9wYXRoKHByb3h5
KSk7DQo+PiArICAgICAgIGlmICghY3RybCkNCj4+ICsgICAgICAgICAgICAgICBjdHJsID0gYWRh
cHRlcl9uZXcocHJveHkpOw0KPj4gKw0KPj4gKyAgICAgICBjdHJsLT5hZF9wcm94eSA9IHByb3h5
Ow0KPj4gIH0NCj4+DQo+PiAgc3RhdGljIHZvaWQgcHJveHlfYWRkZWQoR0RCdXNQcm94eSAqcHJv
eHksIHZvaWQgKnVzZXJfZGF0YSkgQEANCj4+IC02NjEsMjAgKzY5Niw2IEBAIHN0YXRpYyB2b2lk
IHByb3h5X3JlbW92ZWQoR0RCdXNQcm94eSAqcHJveHksIHZvaWQgKnVzZXJfZGF0YSkNCj4+ICAg
ICAgICAgfQ0KPj4gIH0NCj4+DQo+PiAtc3RhdGljIHN0cnVjdCBhZGFwdGVyICpmaW5kX2N0cmwo
R0xpc3QgKnNvdXJjZSwgY29uc3QgY2hhciAqcGF0aCkgLXsNCj4+IC0gICAgICAgR0xpc3QgKmxp
c3Q7DQo+PiAtDQo+PiAtICAgICAgIGZvciAobGlzdCA9IGdfbGlzdF9maXJzdChzb3VyY2UpOyBs
aXN0OyBsaXN0ID0gZ19saXN0X25leHQobGlzdCkpIHsNCj4+IC0gICAgICAgICAgICAgICBzdHJ1
Y3QgYWRhcHRlciAqYWRhcHRlciA9IGxpc3QtPmRhdGE7DQo+PiAtDQo+PiAtICAgICAgICAgICAg
ICAgaWYgKCFzdHJjYXNlY21wKGdfZGJ1c19wcm94eV9nZXRfcGF0aChhZGFwdGVyLT5wcm94eSks
IHBhdGgpKQ0KPj4gLSAgICAgICAgICAgICAgICAgICAgICAgcmV0dXJuIGFkYXB0ZXI7DQo+PiAt
ICAgICAgIH0NCj4+IC0NCj4+IC0gICAgICAgcmV0dXJuIE5VTEw7DQo+PiAtfQ0KPj4gLQ0KPj4g
IHN0YXRpYyB2b2lkIHByb3BlcnR5X2NoYW5nZWQoR0RCdXNQcm94eSAqcHJveHksIGNvbnN0IGNo
YXIgKm5hbWUsDQo+PiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgREJ1
c01lc3NhZ2VJdGVyICppdGVyLCB2b2lkDQo+PiAqdXNlcl9kYXRhKSAgew0KPj4gLS0NCj4+IDIu
Ny40DQo+Pg0KPj4NCj4+IFJlZ2FyZHMsDQo+PiBFcmFtb3RvDQo+Pg0KPj4gT24gMDkvMDEvMjAx
NyAxMjo1NiBQTSwgWXVuaGFuIFdhbmcgd3JvdGU6DQo+Pj4gV2hlbiBhbm90aGVyIGFkYXB0ZXIg
aXMgZm91bmQsIHRoZSBkZWZhdWx0IGFkYXB0ZXIgd291bGQgYmUgY2hhbmdlZCwgDQo+Pj4gd2hp
Y2ggaXMgbm90IGV4cGVjdGVkLiBEZWZhdWx0IGFkYXB0ZXIgY2FuIG9ubHkgYmUgY2hhbmdlZCBi
eSBzZWxlY3QgDQo+Pj4gY29tbWFuZC4NCj4+PiAtLS0NCj4+PiAgY2xpZW50L21haW4uYyB8IDE4
ICsrKysrKysrKysrKy0tLS0tLQ0KPj4+ICAxIGZpbGUgY2hhbmdlZCwgMTIgaW5zZXJ0aW9ucygr
KSwgNiBkZWxldGlvbnMoLSkNCj4+Pg0KPj4+IGRpZmYgLS1naXQgYS9jbGllbnQvbWFpbi5jIGIv
Y2xpZW50L21haW4uYyBpbmRleCANCj4+PiA3NTY5NmMyYzEuLjI5YjM5NzhlMQ0KPj4+IDEwMDY0
NA0KPj4+IC0tLSBhL2NsaWVudC9tYWluLmMNCj4+PiArKysgYi9jbGllbnQvbWFpbi5jDQo+Pj4g
QEAgLTY3LDYgKzY3LDcgQEAgc3RydWN0IGFkYXB0ZXIgew0KPj4+ICB9Ow0KPj4+DQo+Pj4gIHN0
YXRpYyBzdHJ1Y3QgYWRhcHRlciAqZGVmYXVsdF9jdHJsOw0KPj4+ICtzdGF0aWMgc3RydWN0IGFk
YXB0ZXIgKmNhY2hlX2N0cmw7DQo+Pj4gIHN0YXRpYyBHREJ1c1Byb3h5ICpkZWZhdWx0X2RldjsN
Cj4+PiAgc3RhdGljIEdEQnVzUHJveHkgKmRlZmF1bHRfYXR0cjsNCj4+PiAgc3RhdGljIEdMaXN0
ICpjdHJsX2xpc3Q7DQo+Pj4gQEAgLTE1MSw3ICsxNTIsNyBAQCBzdGF0aWMgdm9pZCBkaXNjb25u
ZWN0X2hhbmRsZXIoREJ1c0Nvbm5lY3Rpb24gDQo+Pj4gKmNvbm5lY3Rpb24sIHZvaWQgKnVzZXJf
ZGF0YSkNCj4+Pg0KPj4+ICAgICAgIGdfbGlzdF9mcmVlX2Z1bGwoY3RybF9saXN0LCBwcm94eV9s
ZWFrKTsNCj4+PiAgICAgICBjdHJsX2xpc3QgPSBOVUxMOw0KPj4+IC0NCj4+PiArICAgICBjYWNo
ZV9jdHJsID0gTlVMTDsNCj4+PiAgICAgICBkZWZhdWx0X2N0cmwgPSBOVUxMOw0KPj4+ICB9DQo+
Pj4NCj4+PiBAQCAtNTIxLDE1ICs1MjIsMjEgQEAgc3RhdGljIHZvaWQgZGV2aWNlX2FkZGVkKEdE
QnVzUHJveHkgKnByb3h5KQ0KPj4+DQo+Pj4gIHN0YXRpYyB2b2lkIGFkYXB0ZXJfYWRkZWQoR0RC
dXNQcm94eSAqcHJveHkpICB7DQo+Pj4gLSAgICAgZGVmYXVsdF9jdHJsID0gZ19tYWxsb2MwKHNp
emVvZihzdHJ1Y3QgYWRhcHRlcikpOw0KPj4+IC0gICAgIGRlZmF1bHRfY3RybC0+cHJveHkgPSBw
cm94eTsNCj4+PiAtICAgICBjdHJsX2xpc3QgPSBnX2xpc3RfYXBwZW5kKGN0cmxfbGlzdCwgZGVm
YXVsdF9jdHJsKTsNCj4+PiArICAgICBzdHJ1Y3QgYWRhcHRlciAqYWRhcHRlciA9IGdfbWFsbG9j
MChzaXplb2Yoc3RydWN0IGFkYXB0ZXIpKTsNCj4+PiArDQo+Pj4gKyAgICAgYWRhcHRlci0+cHJv
eHkgPSBwcm94eTsNCj4+PiArICAgICBjYWNoZV9jdHJsID0gYWRhcHRlcjsNCj4+PiArICAgICBj
dHJsX2xpc3QgPSBnX2xpc3RfYXBwZW5kKGN0cmxfbGlzdCwgYWRhcHRlcik7DQo+Pj4gKw0KPj4+
ICsgICAgIGlmICghZGVmYXVsdF9jdHJsKQ0KPj4+ICsgICAgICAgICAgICAgZGVmYXVsdF9jdHJs
ID0gYWRhcHRlcjsNCj4+PiArDQo+Pj4gICAgICAgcHJpbnRfYWRhcHRlcihwcm94eSwgQ09MT1JF
RF9ORVcpOyAgfQ0KPj4+DQo+Pj4gIHN0YXRpYyB2b2lkIGFkX21hbmFnZXJfYWRkZWQoR0RCdXNQ
cm94eSAqcHJveHkpICB7DQo+Pj4gLSAgICAgZGVmYXVsdF9jdHJsLT5hZF9wcm94eSA9IHByb3h5
Ow0KPj4+ICsgICAgIGNhY2hlX2N0cmwtPmFkX3Byb3h5ID0gcHJveHk7DQo+Pj4gIH0NCj4+Pg0K
Pj4+ICBzdGF0aWMgdm9pZCBwcm94eV9hZGRlZChHREJ1c1Byb3h5ICpwcm94eSwgdm9pZCAqdXNl
cl9kYXRhKSBAQA0KPj4+IC02MDIsNyArNjA5LDYgQEAgc3RhdGljIHZvaWQgYWRhcHRlcl9yZW1v
dmVkKEdEQnVzUHJveHkgKnByb3h5KQ0KPj4+ICAgICAgICAgICAgICAgICAgICAgICBwcmludF9h
ZGFwdGVyKHByb3h5LCBDT0xPUkVEX0RFTCk7DQo+Pj4NCj4+PiAgICAgICAgICAgICAgICAgICAg
ICAgaWYgKGRlZmF1bHRfY3RybCAmJiBkZWZhdWx0X2N0cmwtPnByb3h5ID09IHByb3h5KSB7DQo+
Pj4gLSAgICAgICAgICAgICAgICAgICAgICAgICAgICAgZGVmYXVsdF9jdHJsID0gTlVMTDsNCj4+
PiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBzZXRfZGVmYXVsdF9kZXZpY2UoTlVMTCwg
TlVMTCk7DQo+Pj4gICAgICAgICAgICAgICAgICAgICAgIH0NCj4+Pg0KPj4+DQo+Pg0KPiAtLQ0K
PiBUbyB1bnN1YnNjcmliZSBmcm9tIHRoaXMgbGlzdDogc2VuZCB0aGUgbGluZSAidW5zdWJzY3Jp
YmUgDQo+IGxpbnV4LWJsdWV0b290aCIgaW4gdGhlIGJvZHkgb2YgYSBtZXNzYWdlIHRvIG1ham9y
ZG9tb0B2Z2VyLmtlcm5lbC5vcmcgDQo+IE1vcmUgbWFqb3Jkb21vIGluZm8gYXQgIA0KPiBodHRw
czovL25hMDEuc2FmZWxpbmtzLnByb3RlY3Rpb24ub3V0bG9vay5jb20vP3VybD1odHRwJTNBJTJG
JTJGdmdlci5rDQo+IGVybmVsLm9yZyUyRm1ham9yZG9tby1pbmZvLmh0bWwmZGF0YT0wMSU3QzAx
JTdDTWluZy5aaGFuZyU0MGFycmlzLmNvbSUNCj4gN0NkMDE2M2U2YmQ4NTQ0NDZjYWY2ZDA4ZDRm
MTBkMDQ0NCU3Q2YyNzkyOWFkZTU1NDRkNTU4MzdhYzU2MTUxOWMzMDkxJQ0KPiA3QzEmc2RhdGE9
aklpUlRZaG5CNGtkR0FuSVBSMmRQdG04aTZpYWxMUUZmeUdmVVMxQnhFbyUzRCZyZXNlcnZlZD0w
DQo=

2017-09-06 05:42:45

by Yunhan Wang

[permalink] [raw]
Subject: Re: [PATCH BlueZ] client: Fix default_ctrl change when new adapter is found

Hi, Ming

Could you post your command from your peripheral and central? Usually
when you set the attribute with read,write permission in peripheral,
then in central, you can select-attribute the characteristic path, and
read/write.

thanks
best wishes
yunhan

On Fri, Sep 1, 2017 at 2:06 PM, Zhang, Ming <[email protected]> wrote:
> Hello,
>
> Can anybody give a help with BlueZ utility bluetoothctl?
> - "read" and "write" commands seem not working? always report "no attribu=
te selected", using "select-attribute" first doesn't help.
>
> Thanks,
> Ming
>
> -----Original Message-----
> From: [email protected] [mailto:linux-bluetooth-owner=
@vger.kernel.org] On Behalf Of Yunhan Wang
> Sent: Friday, September 01, 2017 12:43 AM
> To: ERAMOTO Masaya <[email protected]>
> Cc: [email protected]
> Subject: Re: [PATCH BlueZ] client: Fix default_ctrl change when new adapt=
er is found
>
> Hi, ERAMOTO
>
> cache_ctrl is not advertising manager, cache_ctrl is temporary adapter th=
at store newest adapter info(see the below structure info). When you use se=
lect, find_ctrl_by_address would filter your preferred adapter by your addr=
ess, then if it is default, return, otherwise, it would update default_ctrl=
.
>
> struct adapter {
> GDBusProxy *proxy;
> GDBusProxy *ad_proxy;
> GList *devices;
> };
>
> when new adapter is found, the proxy handler would add adapter proxy, pro=
file proxy, advertising proxy sequentially, in this procedure, cache_ctrl w=
ould update advertising manager.
>
> thanks
> best wishes
> yunhan
>
> On Fri, Sep 1, 2017 at 12:28 AM, ERAMOTO Masaya <[email protected]=
u.com> wrote:
>> Hi Yunhan,
>>
>> Your patch have a regression, since select command can not update the
>> current advertising manager (cache_ctrl) when it chooses another adapter=
.
>>
>> I think that it is better to use proxy->obj_path below:
>>
>> ---
>> client/main.c | 57
>> +++++++++++++++++++++++++++++++++++++++------------------
>> 1 file changed, 39 insertions(+), 18 deletions(-)
>>
>> diff --git a/client/main.c b/client/main.c index 825647d..c6cb995
>> 100644
>> --- a/client/main.c
>> +++ b/client/main.c
>> @@ -525,17 +525,52 @@ static void device_added(GDBusProxy *proxy)
>> }
>> }
>>
>> +static struct adapter *find_ctrl(GList *source, const char *path) {
>> + GList *list;
>> +
>> + for (list =3D g_list_first(source); list; list =3D g_list_next(l=
ist)) {
>> + struct adapter *adapter =3D list->data;
>> +
>> + if (!strcasecmp(g_dbus_proxy_get_path(adapter->proxy), p=
ath))
>> + return adapter;
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +static struct adapter *adapter_new(GDBusProxy *proxy) {
>> + struct adapter *adapter =3D g_malloc0(sizeof(struct adapter));
>> +
>> + ctrl_list =3D g_list_append(ctrl_list, adapter);
>> +
>> + if (!default_ctrl)
>> + default_ctrl =3D adapter;
>> +
>> + return adapter;
>> +}
>> +
>> static void adapter_added(GDBusProxy *proxy) {
>> - default_ctrl =3D g_malloc0(sizeof(struct adapter));
>> - default_ctrl->proxy =3D proxy;
>> - ctrl_list =3D g_list_append(ctrl_list, default_ctrl);
>> + struct adapter *ctrl;
>> + ctrl =3D find_ctrl(ctrl_list, g_dbus_proxy_get_path(proxy));
>> + if (!ctrl)
>> + ctrl =3D adapter_new(proxy);
>> +
>> + ctrl->proxy =3D proxy;
>> +
>> print_adapter(proxy, COLORED_NEW); }
>>
>> static void ad_manager_added(GDBusProxy *proxy) {
>> - default_ctrl->ad_proxy =3D proxy;
>> + struct adapter *ctrl;
>> + ctrl =3D find_ctrl(ctrl_list, g_dbus_proxy_get_path(proxy));
>> + if (!ctrl)
>> + ctrl =3D adapter_new(proxy);
>> +
>> + ctrl->ad_proxy =3D proxy;
>> }
>>
>> static void proxy_added(GDBusProxy *proxy, void *user_data) @@
>> -661,20 +696,6 @@ static void proxy_removed(GDBusProxy *proxy, void *use=
r_data)
>> }
>> }
>>
>> -static struct adapter *find_ctrl(GList *source, const char *path) -{
>> - GList *list;
>> -
>> - for (list =3D g_list_first(source); list; list =3D g_list_next(l=
ist)) {
>> - struct adapter *adapter =3D list->data;
>> -
>> - if (!strcasecmp(g_dbus_proxy_get_path(adapter->proxy), p=
ath))
>> - return adapter;
>> - }
>> -
>> - return NULL;
>> -}
>> -
>> static void property_changed(GDBusProxy *proxy, const char *name,
>> DBusMessageIter *iter, void
>> *user_data) {
>> --
>> 2.7.4
>>
>>
>> Regards,
>> Eramoto
>>
>> On 09/01/2017 12:56 PM, Yunhan Wang wrote:
>>> When another adapter is found, the default adapter would be changed,
>>> which is not expected. Default adapter can only be changed by select
>>> command.
>>> ---
>>> client/main.c | 18 ++++++++++++------
>>> 1 file changed, 12 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/client/main.c b/client/main.c index 75696c2c1..29b3978e1
>>> 100644
>>> --- a/client/main.c
>>> +++ b/client/main.c
>>> @@ -67,6 +67,7 @@ struct adapter {
>>> };
>>>
>>> static struct adapter *default_ctrl;
>>> +static struct adapter *cache_ctrl;
>>> static GDBusProxy *default_dev;
>>> static GDBusProxy *default_attr;
>>> static GList *ctrl_list;
>>> @@ -151,7 +152,7 @@ static void disconnect_handler(DBusConnection
>>> *connection, void *user_data)
>>>
>>> g_list_free_full(ctrl_list, proxy_leak);
>>> ctrl_list =3D NULL;
>>> -
>>> + cache_ctrl =3D NULL;
>>> default_ctrl =3D NULL;
>>> }
>>>
>>> @@ -521,15 +522,21 @@ static void device_added(GDBusProxy *proxy)
>>>
>>> static void adapter_added(GDBusProxy *proxy) {
>>> - default_ctrl =3D g_malloc0(sizeof(struct adapter));
>>> - default_ctrl->proxy =3D proxy;
>>> - ctrl_list =3D g_list_append(ctrl_list, default_ctrl);
>>> + struct adapter *adapter =3D g_malloc0(sizeof(struct adapter));
>>> +
>>> + adapter->proxy =3D proxy;
>>> + cache_ctrl =3D adapter;
>>> + ctrl_list =3D g_list_append(ctrl_list, adapter);
>>> +
>>> + if (!default_ctrl)
>>> + default_ctrl =3D adapter;
>>> +
>>> print_adapter(proxy, COLORED_NEW); }
>>>
>>> static void ad_manager_added(GDBusProxy *proxy) {
>>> - default_ctrl->ad_proxy =3D proxy;
>>> + cache_ctrl->ad_proxy =3D proxy;
>>> }
>>>
>>> static void proxy_added(GDBusProxy *proxy, void *user_data) @@
>>> -602,7 +609,6 @@ static void adapter_removed(GDBusProxy *proxy)
>>> print_adapter(proxy, COLORED_DEL);
>>>
>>> if (default_ctrl && default_ctrl->proxy =3D=3D pr=
oxy) {
>>> - default_ctrl =3D NULL;
>>> set_default_device(NULL, NULL);
>>> }
>>>
>>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth=
" in the body of a message to [email protected] More majordomo info=
at https://na01.safelinks.protection.outlook.com/?url=3Dhttp%3A%2F%2Fvger=
.kernel.org%2Fmajordomo-info.html&data=3D01%7C01%7CMing.Zhang%40arris.com%7=
Cd0163e6bd854446caf6d08d4f10d0444%7Cf27929ade5544d55837ac561519c3091%7C1&sd=
ata=3DjIiRTYhnB4kdGAnIPR2dPtm8i6ialLQFfyGfUS1BxEo%3D&reserved=3D0

2017-09-06 05:36:18

by Yunhan Wang

[permalink] [raw]
Subject: Re: [PATCH BlueZ] client: Fix default_ctrl change when new adapter is found

Hi, ERAMOTO

On Sun, Sep 3, 2017 at 5:48 PM, ERAMOTO Masaya
<[email protected]> wrote:
> Hi Yunhan,
>
> On 09/02/2017 12:54 PM, Yunhan Wang wrote:
>> Hi, ERAMOTO
>>
>> On Fri, Sep 1, 2017 at 12:59 AM, ERAMOTO Masaya
>> <[email protected]> wrote:
>>> Hi Yunhan,
>>>
>>> Thanks for your explanation.
>>>
>>> Btw, could you also tell me about the following fix.
>>
>> default_ctrl would be set as NULL in disconnect_handler. Therefore I
>> remove this redundant one.
>>
>
> It seems g_dbus_client_unref() calls disconnect_handler() when g_main_loop_run()
> is exited by quit command etc.. So I think here defautl_ctrl should be set as
> NULL in interactive mode.
>

Yes, just send patch to fix it.

>>
>>> bluetoothclt sometimes do core dump when I detach the adapter of default_ctrl
>>> and run show command.
>>
>> Could you share your reproduce step for this issue? Could it be
>> constantly reproduced?
>
> It is always reproduced with the following steps:
>
> 1. Attach an adapter.
> 2. Run bluetoothctl.
> 3. Detach this adapter.
> 4. Run device command in bluetootctl.
>
With the new fix, when you detach adapter, the default_ctrl become
NULL, you should not see the devices via running devices cmd.
>
> Regards,
> Eramoto
>
>>
>> thanks
>> best wishes
>> yunhan
>>
>>
>

thanks
best wishes
yunhan

2017-09-04 00:48:01

by ERAMOTO Masaya

[permalink] [raw]
Subject: Re: [PATCH BlueZ] client: Fix default_ctrl change when new adapter is found

Hi Yunhan,

On 09/02/2017 12:54 PM, Yunhan Wang wrote:
> Hi, ERAMOTO
>
> On Fri, Sep 1, 2017 at 12:59 AM, ERAMOTO Masaya
> <[email protected]> wrote:
>> Hi Yunhan,
>>
>> Thanks for your explanation.
>>
>> Btw, could you also tell me about the following fix.
>
> default_ctrl would be set as NULL in disconnect_handler. Therefore I
> remove this redundant one.
>

It seems g_dbus_client_unref() calls disconnect_handler() when g_main_loop_run()
is exited by quit command etc.. So I think here defautl_ctrl should be set as
NULL in interactive mode.

>
>> bluetoothclt sometimes do core dump when I detach the adapter of default_ctrl
>> and run show command.
>
> Could you share your reproduce step for this issue? Could it be
> constantly reproduced?

It is always reproduced with the following steps:

1. Attach an adapter.
2. Run bluetoothctl.
3. Detach this adapter.
4. Run device command in bluetootctl.


Regards,
Eramoto

>
> thanks
> best wishes
> yunhan
>
>


2017-09-02 03:54:13

by Yunhan Wang

[permalink] [raw]
Subject: Re: [PATCH BlueZ] client: Fix default_ctrl change when new adapter is found

Hi, ERAMOTO

On Fri, Sep 1, 2017 at 12:59 AM, ERAMOTO Masaya
<[email protected]> wrote:
> Hi Yunhan,
>
> Thanks for your explanation.
>
> Btw, could you also tell me about the following fix.

default_ctrl would be set as NULL in disconnect_handler. Therefore I
remove this redundant one.


> bluetoothclt sometimes do core dump when I detach the adapter of default_ctrl
> and run show command.

Could you share your reproduce step for this issue? Could it be
constantly reproduced?

thanks
best wishes
yunhan

2017-09-01 21:06:17

by Zhang, Ming

[permalink] [raw]
Subject: RE: [PATCH BlueZ] client: Fix default_ctrl change when new adapter is found

SGVsbG8sIA0KDQpDYW4gYW55Ym9keSBnaXZlIGEgaGVscCB3aXRoIEJsdWVaIHV0aWxpdHkgYmx1
ZXRvb3RoY3RsPyANCi0gInJlYWQiIGFuZCAid3JpdGUiIGNvbW1hbmRzIHNlZW0gbm90IHdvcmtp
bmc/IGFsd2F5cyByZXBvcnQgIm5vIGF0dHJpYnV0ZSBzZWxlY3RlZCIsIHVzaW5nICJzZWxlY3Qt
YXR0cmlidXRlIiBmaXJzdCBkb2Vzbid0IGhlbHAuIA0KDQpUaGFua3MsIA0KTWluZw0KDQotLS0t
LU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KRnJvbTogbGludXgtYmx1ZXRvb3RoLW93bmVyQHZnZXIu
a2VybmVsLm9yZyBbbWFpbHRvOmxpbnV4LWJsdWV0b290aC1vd25lckB2Z2VyLmtlcm5lbC5vcmdd
IE9uIEJlaGFsZiBPZiBZdW5oYW4gV2FuZw0KU2VudDogRnJpZGF5LCBTZXB0ZW1iZXIgMDEsIDIw
MTcgMTI6NDMgQU0NClRvOiBFUkFNT1RPIE1hc2F5YSA8ZXJhbW90by5tYXNheWFAanAuZnVqaXRz
dS5jb20+DQpDYzogbGludXgtYmx1ZXRvb3RoQHZnZXIua2VybmVsLm9yZw0KU3ViamVjdDogUmU6
IFtQQVRDSCBCbHVlWl0gY2xpZW50OiBGaXggZGVmYXVsdF9jdHJsIGNoYW5nZSB3aGVuIG5ldyBh
ZGFwdGVyIGlzIGZvdW5kDQoNCkhpLCBFUkFNT1RPDQoNCmNhY2hlX2N0cmwgaXMgbm90IGFkdmVy
dGlzaW5nIG1hbmFnZXIsIGNhY2hlX2N0cmwgaXMgdGVtcG9yYXJ5IGFkYXB0ZXIgdGhhdCBzdG9y
ZSBuZXdlc3QgYWRhcHRlciBpbmZvKHNlZSB0aGUgYmVsb3cgc3RydWN0dXJlIGluZm8pLiBXaGVu
IHlvdSB1c2Ugc2VsZWN0LCBmaW5kX2N0cmxfYnlfYWRkcmVzcyB3b3VsZCBmaWx0ZXIgeW91ciBw
cmVmZXJyZWQgYWRhcHRlciBieSB5b3VyIGFkZHJlc3MsIHRoZW4gaWYgaXQgaXMgZGVmYXVsdCwg
cmV0dXJuLCBvdGhlcndpc2UsIGl0IHdvdWxkIHVwZGF0ZSBkZWZhdWx0X2N0cmwuDQoNCnN0cnVj
dCBhZGFwdGVyIHsNCkdEQnVzUHJveHkgKnByb3h5Ow0KR0RCdXNQcm94eSAqYWRfcHJveHk7DQpH
TGlzdCAqZGV2aWNlczsNCn07DQoNCndoZW4gbmV3IGFkYXB0ZXIgaXMgZm91bmQsIHRoZSBwcm94
eSBoYW5kbGVyIHdvdWxkIGFkZCBhZGFwdGVyIHByb3h5LCBwcm9maWxlIHByb3h5LCBhZHZlcnRp
c2luZyBwcm94eSBzZXF1ZW50aWFsbHksIGluIHRoaXMgcHJvY2VkdXJlLCBjYWNoZV9jdHJsIHdv
dWxkIHVwZGF0ZSBhZHZlcnRpc2luZyBtYW5hZ2VyLg0KDQp0aGFua3MNCmJlc3Qgd2lzaGVzDQp5
dW5oYW4NCg0KT24gRnJpLCBTZXAgMSwgMjAxNyBhdCAxMjoyOCBBTSwgRVJBTU9UTyBNYXNheWEg
PGVyYW1vdG8ubWFzYXlhQGpwLmZ1aml0c3UuY29tPiB3cm90ZToNCj4gSGkgWXVuaGFuLA0KPg0K
PiBZb3VyIHBhdGNoIGhhdmUgYSByZWdyZXNzaW9uLCBzaW5jZSBzZWxlY3QgY29tbWFuZCBjYW4g
bm90IHVwZGF0ZSB0aGUgDQo+IGN1cnJlbnQgYWR2ZXJ0aXNpbmcgbWFuYWdlciAoY2FjaGVfY3Ry
bCkgd2hlbiBpdCBjaG9vc2VzIGFub3RoZXIgYWRhcHRlci4NCj4NCj4gSSB0aGluayB0aGF0IGl0
IGlzIGJldHRlciB0byB1c2UgcHJveHktPm9ial9wYXRoIGJlbG93Og0KPg0KPiAtLS0NCj4gIGNs
aWVudC9tYWluLmMgfCA1NyANCj4gKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysr
KysrLS0tLS0tLS0tLS0tLS0tLS0tDQo+ICAxIGZpbGUgY2hhbmdlZCwgMzkgaW5zZXJ0aW9ucygr
KSwgMTggZGVsZXRpb25zKC0pDQo+DQo+IGRpZmYgLS1naXQgYS9jbGllbnQvbWFpbi5jIGIvY2xp
ZW50L21haW4uYyBpbmRleCA4MjU2NDdkLi5jNmNiOTk1IA0KPiAxMDA2NDQNCj4gLS0tIGEvY2xp
ZW50L21haW4uYw0KPiArKysgYi9jbGllbnQvbWFpbi5jDQo+IEBAIC01MjUsMTcgKzUyNSw1MiBA
QCBzdGF0aWMgdm9pZCBkZXZpY2VfYWRkZWQoR0RCdXNQcm94eSAqcHJveHkpDQo+ICAgICAgICAg
fQ0KPiAgfQ0KPg0KPiArc3RhdGljIHN0cnVjdCBhZGFwdGVyICpmaW5kX2N0cmwoR0xpc3QgKnNv
dXJjZSwgY29uc3QgY2hhciAqcGF0aCkgew0KPiArICAgICAgIEdMaXN0ICpsaXN0Ow0KPiArDQo+
ICsgICAgICAgZm9yIChsaXN0ID0gZ19saXN0X2ZpcnN0KHNvdXJjZSk7IGxpc3Q7IGxpc3QgPSBn
X2xpc3RfbmV4dChsaXN0KSkgew0KPiArICAgICAgICAgICAgICAgc3RydWN0IGFkYXB0ZXIgKmFk
YXB0ZXIgPSBsaXN0LT5kYXRhOw0KPiArDQo+ICsgICAgICAgICAgICAgICBpZiAoIXN0cmNhc2Vj
bXAoZ19kYnVzX3Byb3h5X2dldF9wYXRoKGFkYXB0ZXItPnByb3h5KSwgcGF0aCkpDQo+ICsgICAg
ICAgICAgICAgICAgICAgICAgIHJldHVybiBhZGFwdGVyOw0KPiArICAgICAgIH0NCj4gKw0KPiAr
ICAgICAgIHJldHVybiBOVUxMOw0KPiArfQ0KPiArDQo+ICtzdGF0aWMgc3RydWN0IGFkYXB0ZXIg
KmFkYXB0ZXJfbmV3KEdEQnVzUHJveHkgKnByb3h5KSB7DQo+ICsgICAgICAgc3RydWN0IGFkYXB0
ZXIgKmFkYXB0ZXIgPSBnX21hbGxvYzAoc2l6ZW9mKHN0cnVjdCBhZGFwdGVyKSk7DQo+ICsNCj4g
KyAgICAgICBjdHJsX2xpc3QgPSBnX2xpc3RfYXBwZW5kKGN0cmxfbGlzdCwgYWRhcHRlcik7DQo+
ICsNCj4gKyAgICAgICBpZiAoIWRlZmF1bHRfY3RybCkNCj4gKyAgICAgICAgICAgICAgIGRlZmF1
bHRfY3RybCA9IGFkYXB0ZXI7DQo+ICsNCj4gKyAgICAgICByZXR1cm4gYWRhcHRlcjsNCj4gK30N
Cj4gKw0KPiAgc3RhdGljIHZvaWQgYWRhcHRlcl9hZGRlZChHREJ1c1Byb3h5ICpwcm94eSkgIHsN
Cj4gLSAgICAgICBkZWZhdWx0X2N0cmwgPSBnX21hbGxvYzAoc2l6ZW9mKHN0cnVjdCBhZGFwdGVy
KSk7DQo+IC0gICAgICAgZGVmYXVsdF9jdHJsLT5wcm94eSA9IHByb3h5Ow0KPiAtICAgICAgIGN0
cmxfbGlzdCA9IGdfbGlzdF9hcHBlbmQoY3RybF9saXN0LCBkZWZhdWx0X2N0cmwpOw0KPiArICAg
ICAgIHN0cnVjdCBhZGFwdGVyICpjdHJsOw0KPiArICAgICAgIGN0cmwgPSBmaW5kX2N0cmwoY3Ry
bF9saXN0LCBnX2RidXNfcHJveHlfZ2V0X3BhdGgocHJveHkpKTsNCj4gKyAgICAgICBpZiAoIWN0
cmwpDQo+ICsgICAgICAgICAgICAgICBjdHJsID0gYWRhcHRlcl9uZXcocHJveHkpOw0KPiArDQo+
ICsgICAgICAgY3RybC0+cHJveHkgPSBwcm94eTsNCj4gKw0KPiAgICAgICAgIHByaW50X2FkYXB0
ZXIocHJveHksIENPTE9SRURfTkVXKTsgIH0NCj4NCj4gIHN0YXRpYyB2b2lkIGFkX21hbmFnZXJf
YWRkZWQoR0RCdXNQcm94eSAqcHJveHkpICB7DQo+IC0gICAgICAgZGVmYXVsdF9jdHJsLT5hZF9w
cm94eSA9IHByb3h5Ow0KPiArICAgICAgIHN0cnVjdCBhZGFwdGVyICpjdHJsOw0KPiArICAgICAg
IGN0cmwgPSBmaW5kX2N0cmwoY3RybF9saXN0LCBnX2RidXNfcHJveHlfZ2V0X3BhdGgocHJveHkp
KTsNCj4gKyAgICAgICBpZiAoIWN0cmwpDQo+ICsgICAgICAgICAgICAgICBjdHJsID0gYWRhcHRl
cl9uZXcocHJveHkpOw0KPiArDQo+ICsgICAgICAgY3RybC0+YWRfcHJveHkgPSBwcm94eTsNCj4g
IH0NCj4NCj4gIHN0YXRpYyB2b2lkIHByb3h5X2FkZGVkKEdEQnVzUHJveHkgKnByb3h5LCB2b2lk
ICp1c2VyX2RhdGEpIEBAIA0KPiAtNjYxLDIwICs2OTYsNiBAQCBzdGF0aWMgdm9pZCBwcm94eV9y
ZW1vdmVkKEdEQnVzUHJveHkgKnByb3h5LCB2b2lkICp1c2VyX2RhdGEpDQo+ICAgICAgICAgfQ0K
PiAgfQ0KPg0KPiAtc3RhdGljIHN0cnVjdCBhZGFwdGVyICpmaW5kX2N0cmwoR0xpc3QgKnNvdXJj
ZSwgY29uc3QgY2hhciAqcGF0aCkgLXsNCj4gLSAgICAgICBHTGlzdCAqbGlzdDsNCj4gLQ0KPiAt
ICAgICAgIGZvciAobGlzdCA9IGdfbGlzdF9maXJzdChzb3VyY2UpOyBsaXN0OyBsaXN0ID0gZ19s
aXN0X25leHQobGlzdCkpIHsNCj4gLSAgICAgICAgICAgICAgIHN0cnVjdCBhZGFwdGVyICphZGFw
dGVyID0gbGlzdC0+ZGF0YTsNCj4gLQ0KPiAtICAgICAgICAgICAgICAgaWYgKCFzdHJjYXNlY21w
KGdfZGJ1c19wcm94eV9nZXRfcGF0aChhZGFwdGVyLT5wcm94eSksIHBhdGgpKQ0KPiAtICAgICAg
ICAgICAgICAgICAgICAgICByZXR1cm4gYWRhcHRlcjsNCj4gLSAgICAgICB9DQo+IC0NCj4gLSAg
ICAgICByZXR1cm4gTlVMTDsNCj4gLX0NCj4gLQ0KPiAgc3RhdGljIHZvaWQgcHJvcGVydHlfY2hh
bmdlZChHREJ1c1Byb3h5ICpwcm94eSwgY29uc3QgY2hhciAqbmFtZSwNCj4gICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgIERCdXNNZXNzYWdlSXRlciAqaXRlciwgdm9pZCAN
Cj4gKnVzZXJfZGF0YSkgIHsNCj4gLS0NCj4gMi43LjQNCj4NCj4NCj4gUmVnYXJkcywNCj4gRXJh
bW90bw0KPg0KPiBPbiAwOS8wMS8yMDE3IDEyOjU2IFBNLCBZdW5oYW4gV2FuZyB3cm90ZToNCj4+
IFdoZW4gYW5vdGhlciBhZGFwdGVyIGlzIGZvdW5kLCB0aGUgZGVmYXVsdCBhZGFwdGVyIHdvdWxk
IGJlIGNoYW5nZWQsIA0KPj4gd2hpY2ggaXMgbm90IGV4cGVjdGVkLiBEZWZhdWx0IGFkYXB0ZXIg
Y2FuIG9ubHkgYmUgY2hhbmdlZCBieSBzZWxlY3QgDQo+PiBjb21tYW5kLg0KPj4gLS0tDQo+PiAg
Y2xpZW50L21haW4uYyB8IDE4ICsrKysrKysrKysrKy0tLS0tLQ0KPj4gIDEgZmlsZSBjaGFuZ2Vk
LCAxMiBpbnNlcnRpb25zKCspLCA2IGRlbGV0aW9ucygtKQ0KPj4NCj4+IGRpZmYgLS1naXQgYS9j
bGllbnQvbWFpbi5jIGIvY2xpZW50L21haW4uYyBpbmRleCA3NTY5NmMyYzEuLjI5YjM5NzhlMSAN
Cj4+IDEwMDY0NA0KPj4gLS0tIGEvY2xpZW50L21haW4uYw0KPj4gKysrIGIvY2xpZW50L21haW4u
Yw0KPj4gQEAgLTY3LDYgKzY3LDcgQEAgc3RydWN0IGFkYXB0ZXIgew0KPj4gIH07DQo+Pg0KPj4g
IHN0YXRpYyBzdHJ1Y3QgYWRhcHRlciAqZGVmYXVsdF9jdHJsOw0KPj4gK3N0YXRpYyBzdHJ1Y3Qg
YWRhcHRlciAqY2FjaGVfY3RybDsNCj4+ICBzdGF0aWMgR0RCdXNQcm94eSAqZGVmYXVsdF9kZXY7
DQo+PiAgc3RhdGljIEdEQnVzUHJveHkgKmRlZmF1bHRfYXR0cjsNCj4+ICBzdGF0aWMgR0xpc3Qg
KmN0cmxfbGlzdDsNCj4+IEBAIC0xNTEsNyArMTUyLDcgQEAgc3RhdGljIHZvaWQgZGlzY29ubmVj
dF9oYW5kbGVyKERCdXNDb25uZWN0aW9uIA0KPj4gKmNvbm5lY3Rpb24sIHZvaWQgKnVzZXJfZGF0
YSkNCj4+DQo+PiAgICAgICBnX2xpc3RfZnJlZV9mdWxsKGN0cmxfbGlzdCwgcHJveHlfbGVhayk7
DQo+PiAgICAgICBjdHJsX2xpc3QgPSBOVUxMOw0KPj4gLQ0KPj4gKyAgICAgY2FjaGVfY3RybCA9
IE5VTEw7DQo+PiAgICAgICBkZWZhdWx0X2N0cmwgPSBOVUxMOw0KPj4gIH0NCj4+DQo+PiBAQCAt
NTIxLDE1ICs1MjIsMjEgQEAgc3RhdGljIHZvaWQgZGV2aWNlX2FkZGVkKEdEQnVzUHJveHkgKnBy
b3h5KQ0KPj4NCj4+ICBzdGF0aWMgdm9pZCBhZGFwdGVyX2FkZGVkKEdEQnVzUHJveHkgKnByb3h5
KSAgew0KPj4gLSAgICAgZGVmYXVsdF9jdHJsID0gZ19tYWxsb2MwKHNpemVvZihzdHJ1Y3QgYWRh
cHRlcikpOw0KPj4gLSAgICAgZGVmYXVsdF9jdHJsLT5wcm94eSA9IHByb3h5Ow0KPj4gLSAgICAg
Y3RybF9saXN0ID0gZ19saXN0X2FwcGVuZChjdHJsX2xpc3QsIGRlZmF1bHRfY3RybCk7DQo+PiAr
ICAgICBzdHJ1Y3QgYWRhcHRlciAqYWRhcHRlciA9IGdfbWFsbG9jMChzaXplb2Yoc3RydWN0IGFk
YXB0ZXIpKTsNCj4+ICsNCj4+ICsgICAgIGFkYXB0ZXItPnByb3h5ID0gcHJveHk7DQo+PiArICAg
ICBjYWNoZV9jdHJsID0gYWRhcHRlcjsNCj4+ICsgICAgIGN0cmxfbGlzdCA9IGdfbGlzdF9hcHBl
bmQoY3RybF9saXN0LCBhZGFwdGVyKTsNCj4+ICsNCj4+ICsgICAgIGlmICghZGVmYXVsdF9jdHJs
KQ0KPj4gKyAgICAgICAgICAgICBkZWZhdWx0X2N0cmwgPSBhZGFwdGVyOw0KPj4gKw0KPj4gICAg
ICAgcHJpbnRfYWRhcHRlcihwcm94eSwgQ09MT1JFRF9ORVcpOyAgfQ0KPj4NCj4+ICBzdGF0aWMg
dm9pZCBhZF9tYW5hZ2VyX2FkZGVkKEdEQnVzUHJveHkgKnByb3h5KSAgew0KPj4gLSAgICAgZGVm
YXVsdF9jdHJsLT5hZF9wcm94eSA9IHByb3h5Ow0KPj4gKyAgICAgY2FjaGVfY3RybC0+YWRfcHJv
eHkgPSBwcm94eTsNCj4+ICB9DQo+Pg0KPj4gIHN0YXRpYyB2b2lkIHByb3h5X2FkZGVkKEdEQnVz
UHJveHkgKnByb3h5LCB2b2lkICp1c2VyX2RhdGEpIEBAIA0KPj4gLTYwMiw3ICs2MDksNiBAQCBz
dGF0aWMgdm9pZCBhZGFwdGVyX3JlbW92ZWQoR0RCdXNQcm94eSAqcHJveHkpDQo+PiAgICAgICAg
ICAgICAgICAgICAgICAgcHJpbnRfYWRhcHRlcihwcm94eSwgQ09MT1JFRF9ERUwpOw0KPj4NCj4+
ICAgICAgICAgICAgICAgICAgICAgICBpZiAoZGVmYXVsdF9jdHJsICYmIGRlZmF1bHRfY3RybC0+
cHJveHkgPT0gcHJveHkpIHsNCj4+IC0gICAgICAgICAgICAgICAgICAgICAgICAgICAgIGRlZmF1
bHRfY3RybCA9IE5VTEw7DQo+PiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBzZXRfZGVm
YXVsdF9kZXZpY2UoTlVMTCwgTlVMTCk7DQo+PiAgICAgICAgICAgICAgICAgICAgICAgfQ0KPj4N
Cj4+DQo+DQotLQ0KVG8gdW5zdWJzY3JpYmUgZnJvbSB0aGlzIGxpc3Q6IHNlbmQgdGhlIGxpbmUg
InVuc3Vic2NyaWJlIGxpbnV4LWJsdWV0b290aCIgaW4gdGhlIGJvZHkgb2YgYSBtZXNzYWdlIHRv
IG1ham9yZG9tb0B2Z2VyLmtlcm5lbC5vcmcgTW9yZSBtYWpvcmRvbW8gaW5mbyBhdCAgaHR0cHM6
Ly9uYTAxLnNhZmVsaW5rcy5wcm90ZWN0aW9uLm91dGxvb2suY29tLz91cmw9aHR0cCUzQSUyRiUy
RnZnZXIua2VybmVsLm9yZyUyRm1ham9yZG9tby1pbmZvLmh0bWwmZGF0YT0wMSU3QzAxJTdDTWlu
Zy5aaGFuZyU0MGFycmlzLmNvbSU3Q2QwMTYzZTZiZDg1NDQ0NmNhZjZkMDhkNGYxMGQwNDQ0JTdD
ZjI3OTI5YWRlNTU0NGQ1NTgzN2FjNTYxNTE5YzMwOTElN0MxJnNkYXRhPWpJaVJUWWhuQjRrZEdB
bklQUjJkUHRtOGk2aWFsTFFGZnlHZlVTMUJ4RW8lM0QmcmVzZXJ2ZWQ9MA0K

2017-09-01 07:59:23

by ERAMOTO Masaya

[permalink] [raw]
Subject: Re: [PATCH BlueZ] client: Fix default_ctrl change when new adapter is found

Hi Yunhan,

Thanks for your explanation.

Btw, could you also tell me about the following fix.
bluetoothclt sometimes do core dump when I detach the adapter of default_ctrl
and run show command.

>>> @@ -602,7 +609,6 @@ static void adapter_removed(GDBusProxy *proxy)
>>> print_adapter(proxy, COLORED_DEL);
>>>
>>> if (default_ctrl && default_ctrl->proxy == proxy) {
>>> - default_ctrl = NULL;
>>> set_default_device(NULL, NULL);

Regards,
Eramoto

On 09/01/2017 04:42 PM, Yunhan Wang wrote:
> Hi, ERAMOTO
>
> cache_ctrl is not advertising manager, cache_ctrl is temporary adapter
> that store newest adapter info(see the below structure info). When you
> use select, find_ctrl_by_address would filter your preferred adapter
> by your address, then if it is default, return, otherwise, it would
> update default_ctrl.
>
> struct adapter {
> GDBusProxy *proxy;
> GDBusProxy *ad_proxy;
> GList *devices;
> };
>
> when new adapter is found, the proxy handler would add adapter proxy,
> profile proxy, advertising proxy sequentially, in this procedure,
> cache_ctrl would update advertising manager.
>
> thanks
> best wishes
> yunhan
>
> On Fri, Sep 1, 2017 at 12:28 AM, ERAMOTO Masaya
> <[email protected]> wrote:
>> Hi Yunhan,
>>
>> Your patch have a regression, since select command can not update the current
>> advertising manager (cache_ctrl) when it chooses another adapter.
>>
>> I think that it is better to use proxy->obj_path below:
>>
>> ---
>> client/main.c | 57 +++++++++++++++++++++++++++++++++++++++------------------
>> 1 file changed, 39 insertions(+), 18 deletions(-)
>>
>> diff --git a/client/main.c b/client/main.c
>> index 825647d..c6cb995 100644
>> --- a/client/main.c
>> +++ b/client/main.c
>> @@ -525,17 +525,52 @@ static void device_added(GDBusProxy *proxy)
>> }
>> }
>>
>> +static struct adapter *find_ctrl(GList *source, const char *path)
>> +{
>> + GList *list;
>> +
>> + for (list = g_list_first(source); list; list = g_list_next(list)) {
>> + struct adapter *adapter = list->data;
>> +
>> + if (!strcasecmp(g_dbus_proxy_get_path(adapter->proxy), path))
>> + return adapter;
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +static struct adapter *adapter_new(GDBusProxy *proxy)
>> +{
>> + struct adapter *adapter = g_malloc0(sizeof(struct adapter));
>> +
>> + ctrl_list = g_list_append(ctrl_list, adapter);
>> +
>> + if (!default_ctrl)
>> + default_ctrl = adapter;
>> +
>> + return adapter;
>> +}
>> +
>> static void adapter_added(GDBusProxy *proxy)
>> {
>> - default_ctrl = g_malloc0(sizeof(struct adapter));
>> - default_ctrl->proxy = proxy;
>> - ctrl_list = g_list_append(ctrl_list, default_ctrl);
>> + struct adapter *ctrl;
>> + ctrl = find_ctrl(ctrl_list, g_dbus_proxy_get_path(proxy));
>> + if (!ctrl)
>> + ctrl = adapter_new(proxy);
>> +
>> + ctrl->proxy = proxy;
>> +
>> print_adapter(proxy, COLORED_NEW);
>> }
>>
>> static void ad_manager_added(GDBusProxy *proxy)
>> {
>> - default_ctrl->ad_proxy = proxy;
>> + struct adapter *ctrl;
>> + ctrl = find_ctrl(ctrl_list, g_dbus_proxy_get_path(proxy));
>> + if (!ctrl)
>> + ctrl = adapter_new(proxy);
>> +
>> + ctrl->ad_proxy = proxy;
>> }
>>
>> static void proxy_added(GDBusProxy *proxy, void *user_data)
>> @@ -661,20 +696,6 @@ static void proxy_removed(GDBusProxy *proxy, void *user_data)
>> }
>> }
>>
>> -static struct adapter *find_ctrl(GList *source, const char *path)
>> -{
>> - GList *list;
>> -
>> - for (list = g_list_first(source); list; list = g_list_next(list)) {
>> - struct adapter *adapter = list->data;
>> -
>> - if (!strcasecmp(g_dbus_proxy_get_path(adapter->proxy), path))
>> - return adapter;
>> - }
>> -
>> - return NULL;
>> -}
>> -
>> static void property_changed(GDBusProxy *proxy, const char *name,
>> DBusMessageIter *iter, void *user_data)
>> {
>> --
>> 2.7.4
>>
>>
>> Regards,
>> Eramoto
>>
>> On 09/01/2017 12:56 PM, Yunhan Wang wrote:
>>> When another adapter is found, the default adapter would be changed,
>>> which is not expected. Default adapter can only be changed by select
>>> command.
>>> ---
>>> client/main.c | 18 ++++++++++++------
>>> 1 file changed, 12 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/client/main.c b/client/main.c
>>> index 75696c2c1..29b3978e1 100644
>>> --- a/client/main.c
>>> +++ b/client/main.c
>>> @@ -67,6 +67,7 @@ struct adapter {
>>> };
>>>
>>> static struct adapter *default_ctrl;
>>> +static struct adapter *cache_ctrl;
>>> static GDBusProxy *default_dev;
>>> static GDBusProxy *default_attr;
>>> static GList *ctrl_list;
>>> @@ -151,7 +152,7 @@ static void disconnect_handler(DBusConnection *connection, void *user_data)
>>>
>>> g_list_free_full(ctrl_list, proxy_leak);
>>> ctrl_list = NULL;
>>> -
>>> + cache_ctrl = NULL;
>>> default_ctrl = NULL;
>>> }
>>>
>>> @@ -521,15 +522,21 @@ static void device_added(GDBusProxy *proxy)
>>>
>>> static void adapter_added(GDBusProxy *proxy)
>>> {
>>> - default_ctrl = g_malloc0(sizeof(struct adapter));
>>> - default_ctrl->proxy = proxy;
>>> - ctrl_list = g_list_append(ctrl_list, default_ctrl);
>>> + struct adapter *adapter = g_malloc0(sizeof(struct adapter));
>>> +
>>> + adapter->proxy = proxy;
>>> + cache_ctrl = adapter;
>>> + ctrl_list = g_list_append(ctrl_list, adapter);
>>> +
>>> + if (!default_ctrl)
>>> + default_ctrl = adapter;
>>> +
>>> print_adapter(proxy, COLORED_NEW);
>>> }
>>>
>>> static void ad_manager_added(GDBusProxy *proxy)
>>> {
>>> - default_ctrl->ad_proxy = proxy;
>>> + cache_ctrl->ad_proxy = proxy;
>>> }
>>>
>>> static void proxy_added(GDBusProxy *proxy, void *user_data)
>>> @@ -602,7 +609,6 @@ static void adapter_removed(GDBusProxy *proxy)
>>> print_adapter(proxy, COLORED_DEL);
>>>
>>> if (default_ctrl && default_ctrl->proxy == proxy) {
>>> - default_ctrl = NULL;
>>> set_default_device(NULL, NULL);
>>> }
>>>
>>>
>>
>
>


2017-09-01 07:42:33

by Yunhan Wang

[permalink] [raw]
Subject: Re: [PATCH BlueZ] client: Fix default_ctrl change when new adapter is found

Hi, ERAMOTO

cache_ctrl is not advertising manager, cache_ctrl is temporary adapter
that store newest adapter info(see the below structure info). When you
use select, find_ctrl_by_address would filter your preferred adapter
by your address, then if it is default, return, otherwise, it would
update default_ctrl.

struct adapter {
GDBusProxy *proxy;
GDBusProxy *ad_proxy;
GList *devices;
};

when new adapter is found, the proxy handler would add adapter proxy,
profile proxy, advertising proxy sequentially, in this procedure,
cache_ctrl would update advertising manager.

thanks
best wishes
yunhan

On Fri, Sep 1, 2017 at 12:28 AM, ERAMOTO Masaya
<[email protected]> wrote:
> Hi Yunhan,
>
> Your patch have a regression, since select command can not update the current
> advertising manager (cache_ctrl) when it chooses another adapter.
>
> I think that it is better to use proxy->obj_path below:
>
> ---
> client/main.c | 57 +++++++++++++++++++++++++++++++++++++++------------------
> 1 file changed, 39 insertions(+), 18 deletions(-)
>
> diff --git a/client/main.c b/client/main.c
> index 825647d..c6cb995 100644
> --- a/client/main.c
> +++ b/client/main.c
> @@ -525,17 +525,52 @@ static void device_added(GDBusProxy *proxy)
> }
> }
>
> +static struct adapter *find_ctrl(GList *source, const char *path)
> +{
> + GList *list;
> +
> + for (list = g_list_first(source); list; list = g_list_next(list)) {
> + struct adapter *adapter = list->data;
> +
> + if (!strcasecmp(g_dbus_proxy_get_path(adapter->proxy), path))
> + return adapter;
> + }
> +
> + return NULL;
> +}
> +
> +static struct adapter *adapter_new(GDBusProxy *proxy)
> +{
> + struct adapter *adapter = g_malloc0(sizeof(struct adapter));
> +
> + ctrl_list = g_list_append(ctrl_list, adapter);
> +
> + if (!default_ctrl)
> + default_ctrl = adapter;
> +
> + return adapter;
> +}
> +
> static void adapter_added(GDBusProxy *proxy)
> {
> - default_ctrl = g_malloc0(sizeof(struct adapter));
> - default_ctrl->proxy = proxy;
> - ctrl_list = g_list_append(ctrl_list, default_ctrl);
> + struct adapter *ctrl;
> + ctrl = find_ctrl(ctrl_list, g_dbus_proxy_get_path(proxy));
> + if (!ctrl)
> + ctrl = adapter_new(proxy);
> +
> + ctrl->proxy = proxy;
> +
> print_adapter(proxy, COLORED_NEW);
> }
>
> static void ad_manager_added(GDBusProxy *proxy)
> {
> - default_ctrl->ad_proxy = proxy;
> + struct adapter *ctrl;
> + ctrl = find_ctrl(ctrl_list, g_dbus_proxy_get_path(proxy));
> + if (!ctrl)
> + ctrl = adapter_new(proxy);
> +
> + ctrl->ad_proxy = proxy;
> }
>
> static void proxy_added(GDBusProxy *proxy, void *user_data)
> @@ -661,20 +696,6 @@ static void proxy_removed(GDBusProxy *proxy, void *user_data)
> }
> }
>
> -static struct adapter *find_ctrl(GList *source, const char *path)
> -{
> - GList *list;
> -
> - for (list = g_list_first(source); list; list = g_list_next(list)) {
> - struct adapter *adapter = list->data;
> -
> - if (!strcasecmp(g_dbus_proxy_get_path(adapter->proxy), path))
> - return adapter;
> - }
> -
> - return NULL;
> -}
> -
> static void property_changed(GDBusProxy *proxy, const char *name,
> DBusMessageIter *iter, void *user_data)
> {
> --
> 2.7.4
>
>
> Regards,
> Eramoto
>
> On 09/01/2017 12:56 PM, Yunhan Wang wrote:
>> When another adapter is found, the default adapter would be changed,
>> which is not expected. Default adapter can only be changed by select
>> command.
>> ---
>> client/main.c | 18 ++++++++++++------
>> 1 file changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/client/main.c b/client/main.c
>> index 75696c2c1..29b3978e1 100644
>> --- a/client/main.c
>> +++ b/client/main.c
>> @@ -67,6 +67,7 @@ struct adapter {
>> };
>>
>> static struct adapter *default_ctrl;
>> +static struct adapter *cache_ctrl;
>> static GDBusProxy *default_dev;
>> static GDBusProxy *default_attr;
>> static GList *ctrl_list;
>> @@ -151,7 +152,7 @@ static void disconnect_handler(DBusConnection *connection, void *user_data)
>>
>> g_list_free_full(ctrl_list, proxy_leak);
>> ctrl_list = NULL;
>> -
>> + cache_ctrl = NULL;
>> default_ctrl = NULL;
>> }
>>
>> @@ -521,15 +522,21 @@ static void device_added(GDBusProxy *proxy)
>>
>> static void adapter_added(GDBusProxy *proxy)
>> {
>> - default_ctrl = g_malloc0(sizeof(struct adapter));
>> - default_ctrl->proxy = proxy;
>> - ctrl_list = g_list_append(ctrl_list, default_ctrl);
>> + struct adapter *adapter = g_malloc0(sizeof(struct adapter));
>> +
>> + adapter->proxy = proxy;
>> + cache_ctrl = adapter;
>> + ctrl_list = g_list_append(ctrl_list, adapter);
>> +
>> + if (!default_ctrl)
>> + default_ctrl = adapter;
>> +
>> print_adapter(proxy, COLORED_NEW);
>> }
>>
>> static void ad_manager_added(GDBusProxy *proxy)
>> {
>> - default_ctrl->ad_proxy = proxy;
>> + cache_ctrl->ad_proxy = proxy;
>> }
>>
>> static void proxy_added(GDBusProxy *proxy, void *user_data)
>> @@ -602,7 +609,6 @@ static void adapter_removed(GDBusProxy *proxy)
>> print_adapter(proxy, COLORED_DEL);
>>
>> if (default_ctrl && default_ctrl->proxy == proxy) {
>> - default_ctrl = NULL;
>> set_default_device(NULL, NULL);
>> }
>>
>>
>

2017-09-01 07:28:39

by ERAMOTO Masaya

[permalink] [raw]
Subject: Re: [PATCH BlueZ] client: Fix default_ctrl change when new adapter is found

Hi Yunhan,

Your patch have a regression, since select command can not update the current
advertising manager (cache_ctrl) when it chooses another adapter.

I think that it is better to use proxy->obj_path below:

---
client/main.c | 57 +++++++++++++++++++++++++++++++++++++++------------------
1 file changed, 39 insertions(+), 18 deletions(-)

diff --git a/client/main.c b/client/main.c
index 825647d..c6cb995 100644
--- a/client/main.c
+++ b/client/main.c
@@ -525,17 +525,52 @@ static void device_added(GDBusProxy *proxy)
}
}

+static struct adapter *find_ctrl(GList *source, const char *path)
+{
+ GList *list;
+
+ for (list = g_list_first(source); list; list = g_list_next(list)) {
+ struct adapter *adapter = list->data;
+
+ if (!strcasecmp(g_dbus_proxy_get_path(adapter->proxy), path))
+ return adapter;
+ }
+
+ return NULL;
+}
+
+static struct adapter *adapter_new(GDBusProxy *proxy)
+{
+ struct adapter *adapter = g_malloc0(sizeof(struct adapter));
+
+ ctrl_list = g_list_append(ctrl_list, adapter);
+
+ if (!default_ctrl)
+ default_ctrl = adapter;
+
+ return adapter;
+}
+
static void adapter_added(GDBusProxy *proxy)
{
- default_ctrl = g_malloc0(sizeof(struct adapter));
- default_ctrl->proxy = proxy;
- ctrl_list = g_list_append(ctrl_list, default_ctrl);
+ struct adapter *ctrl;
+ ctrl = find_ctrl(ctrl_list, g_dbus_proxy_get_path(proxy));
+ if (!ctrl)
+ ctrl = adapter_new(proxy);
+
+ ctrl->proxy = proxy;
+
print_adapter(proxy, COLORED_NEW);
}

static void ad_manager_added(GDBusProxy *proxy)
{
- default_ctrl->ad_proxy = proxy;
+ struct adapter *ctrl;
+ ctrl = find_ctrl(ctrl_list, g_dbus_proxy_get_path(proxy));
+ if (!ctrl)
+ ctrl = adapter_new(proxy);
+
+ ctrl->ad_proxy = proxy;
}

static void proxy_added(GDBusProxy *proxy, void *user_data)
@@ -661,20 +696,6 @@ static void proxy_removed(GDBusProxy *proxy, void *user_data)
}
}

-static struct adapter *find_ctrl(GList *source, const char *path)
-{
- GList *list;
-
- for (list = g_list_first(source); list; list = g_list_next(list)) {
- struct adapter *adapter = list->data;
-
- if (!strcasecmp(g_dbus_proxy_get_path(adapter->proxy), path))
- return adapter;
- }
-
- return NULL;
-}
-
static void property_changed(GDBusProxy *proxy, const char *name,
DBusMessageIter *iter, void *user_data)
{
--
2.7.4


Regards,
Eramoto

On 09/01/2017 12:56 PM, Yunhan Wang wrote:
> When another adapter is found, the default adapter would be changed,
> which is not expected. Default adapter can only be changed by select
> command.
> ---
> client/main.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/client/main.c b/client/main.c
> index 75696c2c1..29b3978e1 100644
> --- a/client/main.c
> +++ b/client/main.c
> @@ -67,6 +67,7 @@ struct adapter {
> };
>
> static struct adapter *default_ctrl;
> +static struct adapter *cache_ctrl;
> static GDBusProxy *default_dev;
> static GDBusProxy *default_attr;
> static GList *ctrl_list;
> @@ -151,7 +152,7 @@ static void disconnect_handler(DBusConnection *connection, void *user_data)
>
> g_list_free_full(ctrl_list, proxy_leak);
> ctrl_list = NULL;
> -
> + cache_ctrl = NULL;
> default_ctrl = NULL;
> }
>
> @@ -521,15 +522,21 @@ static void device_added(GDBusProxy *proxy)
>
> static void adapter_added(GDBusProxy *proxy)
> {
> - default_ctrl = g_malloc0(sizeof(struct adapter));
> - default_ctrl->proxy = proxy;
> - ctrl_list = g_list_append(ctrl_list, default_ctrl);
> + struct adapter *adapter = g_malloc0(sizeof(struct adapter));
> +
> + adapter->proxy = proxy;
> + cache_ctrl = adapter;
> + ctrl_list = g_list_append(ctrl_list, adapter);
> +
> + if (!default_ctrl)
> + default_ctrl = adapter;
> +
> print_adapter(proxy, COLORED_NEW);
> }
>
> static void ad_manager_added(GDBusProxy *proxy)
> {
> - default_ctrl->ad_proxy = proxy;
> + cache_ctrl->ad_proxy = proxy;
> }
>
> static void proxy_added(GDBusProxy *proxy, void *user_data)
> @@ -602,7 +609,6 @@ static void adapter_removed(GDBusProxy *proxy)
> print_adapter(proxy, COLORED_DEL);
>
> if (default_ctrl && default_ctrl->proxy == proxy) {
> - default_ctrl = NULL;
> set_default_device(NULL, NULL);
> }
>
>