* [PATCH 1/3] Fix handling of suspend response
@ 2011-04-04 9:00 Luiz Augusto von Dentz
2011-04-04 9:00 ` [PATCH 2/3] Refactore a2dp finalize_*_errno functions Luiz Augusto von Dentz
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2011-04-04 9:00 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
finalize_suspend is being called twice when a resume request is queue
after it or if start fails.
---
audio/a2dp.c | 22 +++++++++++-----------
1 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/audio/a2dp.c b/audio/a2dp.c
index 9d9c61d..1208ad1 100644
--- a/audio/a2dp.c
+++ b/audio/a2dp.c
@@ -1011,6 +1011,7 @@ static void suspend_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
struct a2dp_sep *a2dp_sep = user_data;
struct a2dp_setup *setup;
gboolean start;
+ int perr;
if (a2dp_sep->type == AVDTP_SEP_TYPE_SINK)
DBG("Sink %p: Suspend_Cfm", sep);
@@ -1029,23 +1030,22 @@ static void suspend_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
if (err) {
setup->stream = NULL;
setup->err = err;
- finalize_suspend(setup);
}
- else
- finalize_suspend_errno(setup, 0);
+
+ finalize_suspend(setup);
if (!start)
return;
if (err) {
- setup->err = err;
- finalize_suspend(setup);
- } else if (avdtp_start(session, a2dp_sep->stream) < 0) {
- struct avdtp_error start_err;
- error("avdtp_start failed");
- avdtp_error_init(&start_err, AVDTP_ERRNO, EIO);
- setup->err = err;
- finalize_suspend(setup);
+ finalize_resume(setup);
+ return;
+ }
+
+ perr = avdtp_start(session, a2dp_sep->stream);
+ if (perr < 0) {
+ error("Error on avdtp_start %s (%d)", strerror(-perr), -perr);
+ finalize_resume_errno(setup, perr);
}
}
--
1.7.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/3] Refactore a2dp finalize_*_errno functions
2011-04-04 9:00 [PATCH 1/3] Fix handling of suspend response Luiz Augusto von Dentz
@ 2011-04-04 9:00 ` Luiz Augusto von Dentz
2011-04-04 9:00 ` [PATCH 3/3] Fix not removing source when removing setup callback Luiz Augusto von Dentz
2011-04-05 2:53 ` [PATCH 1/3] Fix handling of suspend response Johan Hedberg
2 siblings, 0 replies; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2011-04-04 9:00 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
There were a lot of duplicate code in them so they are now replaced by
finalize_setup_errno which can deal with multiple callbacks reusing the
same error.
---
audio/a2dp.c | 88 +++++++++++++++++++++++++++-------------------------------
1 files changed, 41 insertions(+), 47 deletions(-)
diff --git a/audio/a2dp.c b/audio/a2dp.c
index 1208ad1..d51276d 100644
--- a/audio/a2dp.c
+++ b/audio/a2dp.c
@@ -202,8 +202,32 @@ static void setup_cb_free(struct a2dp_setup_cb *cb)
g_free(cb);
}
-static gboolean finalize_config(struct a2dp_setup *s)
+static void finalize_setup_errno(struct a2dp_setup *s, int err,
+ GSourceFunc cb1, ...)
{
+ GSourceFunc finalize;
+ va_list args;
+ struct avdtp_error avdtp_err;
+
+ if (err < 0) {
+ avdtp_error_init(&avdtp_err, AVDTP_ERRNO, -err);
+ s->err = &avdtp_err;
+ }
+
+ va_start(args, cb1);
+ finalize = cb1;
+ setup_ref(s);
+ while (finalize != NULL) {
+ finalize(s);
+ finalize = va_arg(args, GSourceFunc);
+ }
+ setup_unref(s);
+ va_end(args);
+}
+
+static gboolean finalize_config(gpointer data)
+{
+ struct a2dp_setup *s = data;
GSList *l;
struct avdtp_stream *stream = s->err ? NULL : s->stream;
@@ -223,18 +247,9 @@ static gboolean finalize_config(struct a2dp_setup *s)
return FALSE;
}
-static gboolean finalize_config_errno(struct a2dp_setup *s, int err)
-{
- struct avdtp_error avdtp_err;
-
- avdtp_error_init(&avdtp_err, AVDTP_ERRNO, -err);
- s->err = err ? &avdtp_err : NULL;
-
- return finalize_config(s);
-}
-
-static gboolean finalize_resume(struct a2dp_setup *s)
+static gboolean finalize_resume(gpointer data)
{
+ struct a2dp_setup *s = data;
GSList *l;
for (l = s->cb; l != NULL; ) {
@@ -252,18 +267,9 @@ static gboolean finalize_resume(struct a2dp_setup *s)
return FALSE;
}
-static gboolean finalize_resume_errno(struct a2dp_setup *s, int err)
-{
- struct avdtp_error avdtp_err;
-
- avdtp_error_init(&avdtp_err, AVDTP_ERRNO, -err);
- s->err = err ? &avdtp_err : NULL;
-
- return finalize_resume(s);
-}
-
-static gboolean finalize_suspend(struct a2dp_setup *s)
+static gboolean finalize_suspend(gpointer data)
{
+ struct a2dp_setup *s = data;
GSList *l;
for (l = s->cb; l != NULL; ) {
@@ -281,17 +287,7 @@ static gboolean finalize_suspend(struct a2dp_setup *s)
return FALSE;
}
-static gboolean finalize_suspend_errno(struct a2dp_setup *s, int err)
-{
- struct avdtp_error avdtp_err;
-
- avdtp_error_init(&avdtp_err, AVDTP_ERRNO, -err);
- s->err = err ? &avdtp_err : NULL;
-
- return finalize_suspend(s);
-}
-
-static gboolean finalize_select(struct a2dp_setup *s)
+static void finalize_select(struct a2dp_setup *s)
{
GSList *l;
@@ -306,8 +302,6 @@ static gboolean finalize_select(struct a2dp_setup *s)
cb->select_cb(s->session, s->sep, s->caps, cb->user_data);
setup_cb_free(cb);
}
-
- return FALSE;
}
static struct a2dp_setup *find_setup_by_session(struct avdtp *session)
@@ -777,7 +771,7 @@ static void endpoint_open_cb(struct media_endpoint *endpoint, void *ret,
if (ret == NULL) {
setup->stream = NULL;
- finalize_config_errno(setup, -EPERM);
+ finalize_setup_errno(setup, -EPERM, finalize_config);
return;
}
@@ -787,7 +781,7 @@ static void endpoint_open_cb(struct media_endpoint *endpoint, void *ret,
error("Error on avdtp_open %s (%d)", strerror(-err), -err);
setup->stream = NULL;
- finalize_config_errno(setup, err);
+ finalize_setup_errno(setup, err, finalize_config);
}
static void setconf_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
@@ -844,7 +838,7 @@ static void setconf_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
return;
setup->stream = NULL;
- finalize_config_errno(setup, -EPERM);
+ finalize_setup_errno(setup, -EPERM, finalize_config);
return;
}
@@ -852,7 +846,7 @@ static void setconf_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
if (ret < 0) {
error("Error on avdtp_open %s (%d)", strerror(-ret), -ret);
setup->stream = NULL;
- finalize_config_errno(setup, ret);
+ finalize_setup_errno(setup, ret, finalize_config);
}
}
@@ -1045,7 +1039,7 @@ static void suspend_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
perr = avdtp_start(session, a2dp_sep->stream);
if (perr < 0) {
error("Error on avdtp_start %s (%d)", strerror(-perr), -perr);
- finalize_resume_errno(setup, perr);
+ finalize_setup_errno(setup, -EIO, finalize_suspend);
}
}
@@ -1065,8 +1059,8 @@ static gboolean close_ind(struct avdtp *session, struct avdtp_local_sep *sep,
if (!setup)
return TRUE;
- finalize_suspend_errno(setup, -ECONNRESET);
- finalize_resume_errno(setup, -ECONNRESET);
+ finalize_setup_errno(setup, -ECONNRESET, finalize_suspend,
+ finalize_resume);
return TRUE;
}
@@ -1099,7 +1093,7 @@ static gboolean a2dp_reconfigure(gpointer data)
return FALSE;
failed:
- finalize_config_errno(setup, posix_err);
+ finalize_setup_errno(setup, posix_err, finalize_config);
return FALSE;
}
@@ -2126,7 +2120,7 @@ unsigned int a2dp_config(struct avdtp *session, struct a2dp_sep *sep,
case AVDTP_STATE_STREAMING:
if (avdtp_stream_has_capabilities(setup->stream, caps)) {
DBG("Configuration match: resuming");
- g_idle_add((GSourceFunc) finalize_config, setup);
+ g_idle_add(finalize_config, setup);
} else if (!setup->reconfigure) {
setup->reconfigure = TRUE;
if (avdtp_close(session, sep->stream, FALSE) < 0) {
@@ -2184,7 +2178,7 @@ unsigned int a2dp_resume(struct avdtp *session, struct a2dp_sep *sep,
if (sep->suspending)
setup->start = TRUE;
else
- g_idle_add((GSourceFunc) finalize_resume, setup);
+ g_idle_add(finalize_resume, setup);
break;
default:
error("SEP in bad state for resume");
@@ -2221,7 +2215,7 @@ unsigned int a2dp_suspend(struct avdtp *session, struct a2dp_sep *sep,
goto failed;
break;
case AVDTP_STATE_OPEN:
- g_idle_add((GSourceFunc) finalize_suspend, setup);
+ g_idle_add(finalize_suspend, setup);
break;
case AVDTP_STATE_STREAMING:
if (avdtp_suspend(session, sep->stream) < 0) {
--
1.7.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 3/3] Fix not removing source when removing setup callback
2011-04-04 9:00 [PATCH 1/3] Fix handling of suspend response Luiz Augusto von Dentz
2011-04-04 9:00 ` [PATCH 2/3] Refactore a2dp finalize_*_errno functions Luiz Augusto von Dentz
@ 2011-04-04 9:00 ` Luiz Augusto von Dentz
2011-04-05 2:53 ` [PATCH 1/3] Fix handling of suspend response Johan Hedberg
2 siblings, 0 replies; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2011-04-04 9:00 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
In rare situation this may lead to access invalid memory since setup can
be freed before idle callback is called.
---
audio/a2dp.c | 12 +++++++++---
1 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/audio/a2dp.c b/audio/a2dp.c
index d51276d..1f3bc99 100644
--- a/audio/a2dp.c
+++ b/audio/a2dp.c
@@ -82,6 +82,7 @@ struct a2dp_setup_cb {
a2dp_config_cb_t config_cb;
a2dp_stream_cb_t resume_cb;
a2dp_stream_cb_t suspend_cb;
+ guint source_id;
void *user_data;
unsigned int id;
};
@@ -197,6 +198,9 @@ static void setup_cb_free(struct a2dp_setup_cb *cb)
{
struct a2dp_setup *setup = cb->setup;
+ if (cb->source_id)
+ g_source_remove(cb->source_id);
+
setup->cb = g_slist_remove(setup->cb, cb);
setup_unref(cb->setup);
g_free(cb);
@@ -2120,7 +2124,8 @@ unsigned int a2dp_config(struct avdtp *session, struct a2dp_sep *sep,
case AVDTP_STATE_STREAMING:
if (avdtp_stream_has_capabilities(setup->stream, caps)) {
DBG("Configuration match: resuming");
- g_idle_add(finalize_config, setup);
+ cb_data->source_id = g_idle_add(finalize_config,
+ setup);
} else if (!setup->reconfigure) {
setup->reconfigure = TRUE;
if (avdtp_close(session, sep->stream, FALSE) < 0) {
@@ -2178,7 +2183,8 @@ unsigned int a2dp_resume(struct avdtp *session, struct a2dp_sep *sep,
if (sep->suspending)
setup->start = TRUE;
else
- g_idle_add(finalize_resume, setup);
+ cb_data->source_id = g_idle_add(finalize_resume,
+ setup);
break;
default:
error("SEP in bad state for resume");
@@ -2215,7 +2221,7 @@ unsigned int a2dp_suspend(struct avdtp *session, struct a2dp_sep *sep,
goto failed;
break;
case AVDTP_STATE_OPEN:
- g_idle_add(finalize_suspend, setup);
+ cb_data->source_id = g_idle_add(finalize_suspend, setup);
break;
case AVDTP_STATE_STREAMING:
if (avdtp_suspend(session, sep->stream) < 0) {
--
1.7.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/3] Fix handling of suspend response
2011-04-04 9:00 [PATCH 1/3] Fix handling of suspend response Luiz Augusto von Dentz
2011-04-04 9:00 ` [PATCH 2/3] Refactore a2dp finalize_*_errno functions Luiz Augusto von Dentz
2011-04-04 9:00 ` [PATCH 3/3] Fix not removing source when removing setup callback Luiz Augusto von Dentz
@ 2011-04-05 2:53 ` Johan Hedberg
2 siblings, 0 replies; 4+ messages in thread
From: Johan Hedberg @ 2011-04-05 2:53 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
Hi Luiz,
On Mon, Apr 04, 2011, Luiz Augusto von Dentz wrote:
> finalize_suspend is being called twice when a resume request is queue
> after it or if start fails.
> ---
> audio/a2dp.c | 22 +++++++++++-----------
> 1 files changed, 11 insertions(+), 11 deletions(-)
All three patches have been pushed upstream. Thanks.
Johan
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-04-05 2:53 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-04-04 9:00 [PATCH 1/3] Fix handling of suspend response Luiz Augusto von Dentz
2011-04-04 9:00 ` [PATCH 2/3] Refactore a2dp finalize_*_errno functions Luiz Augusto von Dentz
2011-04-04 9:00 ` [PATCH 3/3] Fix not removing source when removing setup callback Luiz Augusto von Dentz
2011-04-05 2:53 ` [PATCH 1/3] Fix handling of suspend response Johan Hedberg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox