2012-03-13 09:26:11

by Mikel Astiz

Subject: [PATCH v0 1/2] audio: fix missing unref in case of error

From: Mikel Astiz <[email protected]>

audio_adapter_get() increases the reference counter of the adapter, so
it's necessary to decrement it in case of error.
---
audio/manager.c | 22 +++++++++++++++++++---
1 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/audio/manager.c b/audio/manager.c
index b984c33..0e09f2f 100644
--- a/audio/manager.c
+++ b/audio/manager.c
@@ -877,6 +877,7 @@ static void audio_adapter_unref(struct audio_adapter *adp)
return;

adapters = g_slist_remove(adapters, adp);
+
btd_adapter_unref(adp->btd_adapter);
g_free(adp);
}
@@ -1014,12 +1015,17 @@ static void headset_server_remove(struct btd_adapter *adapter)
static int gateway_server_probe(struct btd_adapter *adapter)
{
struct audio_adapter *adp;
+ int err;

adp = audio_adapter_get(adapter);
if (!adp)
return -EINVAL;

- return gateway_server_init(adp);
+ err = gateway_server_init(adp);
+ if (err < 0)
+ audio_adapter_unref(adp);
+
+ return err;
}

static void gateway_server_remove(struct btd_adapter *adapter)
@@ -1090,6 +1096,7 @@ static int avrcp_server_probe(struct btd_adapter *adapter)
struct audio_adapter *adp;
const gchar *path = adapter_get_path(adapter);
bdaddr_t src;
+ int err;

DBG("path %s", path);

@@ -1099,7 +1106,11 @@ static int avrcp_server_probe(struct btd_adapter *adapter)

adapter_get_address(adapter, &src);

- return avrcp_register(connection, &src, config);
+ err = avrcp_register(connection, &src, config);
+ if (err < 0)
+ audio_adapter_unref(adp);
+
+ return err;
}

static void avrcp_server_remove(struct btd_adapter *adapter)
@@ -1124,6 +1135,7 @@ static int media_server_probe(struct btd_adapter *adapter)
struct audio_adapter *adp;
const gchar *path = adapter_get_path(adapter);
bdaddr_t src;
+ int err;

DBG("path %s", path);

@@ -1133,7 +1145,11 @@ static int media_server_probe(struct btd_adapter *adapter)

adapter_get_address(adapter, &src);

- return media_register(connection, path, &src);
+ err = media_register(connection, path, &src);
+ if (err < 0)
+ audio_adapter_unref(adp);
+
+ return err;
}

static void media_server_remove(struct btd_adapter *adapter)
--
1.7.7.6

2012-03-13 13:13:16

by Mikel Astiz

Subject: RE: [PATCH v0 1/2] audio: fix missing unref in case of error

SGkgSm9oYW4sDQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogSm9oYW4g
SGVkYmVyZyBbbWFpbHRvOmpvaGFuLmhlZGJlcmdAZ21haWwuY29tXQ0KPiBTZW50OiBEaWVuc3Rh
ZywgMTMuIE3DpHJ6IDIwMTIgMTI6NTUNCj4gVG86IE1pa2VsIEFzdGl6DQo+IENjOiBsaW51eC1i
bHVldG9vdGhAdmdlci5rZXJuZWwub3JnOyBNaWtlbCBBc3Rpeg0KPiBTdWJqZWN0OiBSZTogW1BB
VENIIHYwIDEvMl0gYXVkaW86IGZpeCBtaXNzaW5nIHVucmVmIGluIGNhc2Ugb2YgZXJyb3INCj4g
DQo+IEhpIE1pa2VsLA0KPiANCj4gT24gVHVlLCBNYXIgMTMsIDIwMTIsIE1pa2VsIEFzdGl6IHdy
b3RlOg0KPiA+IEZyb206IE1pa2VsIEFzdGl6IDxtaWtlbC5hc3RpekBibXctY2FyaXQuZGU+DQo+
ID4NCj4gPiBhdWRpb19hZGFwdGVyX2dldCgpIGluY3JlYXNlcyB0aGUgcmVmZXJlbmNlIGNvdW50
ZXIgb2YgdGhlIGFkYXB0ZXIsDQo+IHNvDQo+ID4gaXQncyBuZWNlc3NhcnkgdG8gZGVjcmVtZW50
IGl0IGluIGNhc2Ugb2YgZXJyb3IuDQo+ID4gLS0tDQo+ID4gIGF1ZGlvL21hbmFnZXIuYyB8ICAg
MjIgKysrKysrKysrKysrKysrKysrKy0tLQ0KPiA+ICAxIGZpbGVzIGNoYW5nZWQsIDE5IGluc2Vy
dGlvbnMoKyksIDMgZGVsZXRpb25zKC0pDQo+IA0KPiBQbGVhc2UgdGVzdCBhdCBsZWFzdCB0aGF0
IHlvdXIgcGF0Y2hlcyBjb21waWxlIGJlZm9yZSBzdWJtaXR0aW5nIHRoZW06DQo+IA0KDQpUaGUg
cGF0Y2hlcyBkaWQgbm90IGFwcGx5IHByb3Blcmx5IHRvIHRoZSBtYXN0ZXIgYnJhbmNoLCB3aGlj
aCAoZm9ydHVuYXRlbHkpIGxlZCB0byBjb21waWxhdGlvbiBlcnJvcnMuIFNvcnJ5IGZvciB0aGUg
bWVzcy4NCg0KSSdsbCBzZW5kIGFuIHVwZGF0ZWQgdmVyc2lvbiBzb29uLg0KDQpDaGVlcnMsDQpN
aWtlbA0KDQo=

2012-03-13 11:55:15

by Johan Hedberg

Subject: Re: [PATCH v0 1/2] audio: fix missing unref in case of error

Hi Mikel,

On Tue, Mar 13, 2012, Mikel Astiz wrote:
> From: Mikel Astiz <[email protected]>
>
> audio_adapter_get() increases the reference counter of the adapter, so
> it's necessary to decrement it in case of error.
> ---
> audio/manager.c | 22 +++++++++++++++++++---
> 1 files changed, 19 insertions(+), 3 deletions(-)

Please test at least that your patches compile before submitting them:

CC audio/bluetoothd-manager.o
audio/manager.c: In function ‘avrcp_server_probe’:
audio/manager.c:1058:2: error: ‘err’ undeclared (first use in this function)
audio/manager.c:1058:2: note: each undeclared identifier is reported only once for each function it appears in
audio/manager.c: In function ‘avrcp_server_remove’:
audio/manager.c:1070:6: error: unused variable ‘err’ [-Werror=unused-variable]
audio/manager.c: In function ‘avrcp_server_probe’:
audio/manager.c:1063:1: error: control reaches end of non-void function [-Werror=return-type]

Also:

> --- a/audio/manager.c
> +++ b/audio/manager.c
> @@ -877,6 +877,7 @@ static void audio_adapter_unref(struct audio_adapter *adp)
> return;
>
> adapters = g_slist_remove(adapters, adp);
> +
> btd_adapter_unref(adp->btd_adapter);
> g_free(adp);
> }

This added line seems completely unrelated to the patch.

Johan

2012-03-13 09:26:12

by Mikel Astiz

Subject: [PATCH v0 2/2] audio: fix missing io channel unref if error

From: Mikel Astiz <[email protected]>

In the unlikely case of service record allocation error, the io channel
should be dereferenced.
---
audio/manager.c | 11 +++++++----
1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/audio/manager.c b/audio/manager.c
index 0e09f2f..ddefd41 100644
--- a/audio/manager.c
+++ b/audio/manager.c
@@ -763,20 +763,23 @@ static int gateway_server_init(struct audio_adapter *adapter)
record = hfp_hs_record(chan);
if (!record) {
error("Unable to allocate new service record");
- return -1;
+ goto failed;
}

if (add_record_to_server(&src, record) < 0) {
error("Unable to register HFP HS service record");
sdp_record_free(record);
- g_io_channel_unref(adapter->hfp_hs_server);
- adapter->hfp_hs_server = NULL;
- return -1;
+ goto failed;
}

adapter->hfp_hs_record_id = record->handle;

return 0;
+
+failed:
+ g_io_channel_unref(adapter->hfp_hs_server);
+ adapter->hfp_hs_server = NULL;
+ return -1;
}

static int audio_probe(struct btd_device *device, GSList *uuids)
--
1.7.7.6