Hi Pali,
On Sun, Apr 19, 2020 at 2:14 PM Pali Rohár <pali@xxxxxxxxxx> wrote:
>
> Some bluetooth headsets do not support connecting more then one bluetooth
> profile (e.g. in parallel A2DP and HSP, or HSP and HFP) and issuing
> connect() syscall for second profile returns just ECONNREFUSED.
>
> Prior this patch bluetooth daemon for such situation reported following
> message to log:
>
> Unable to get connect data for Headset Voice gateway: getpeername: Transport endpoint is not connected (107)
>
> Message is incorrect as connect() syscall failed, not getpeername(). This
> patch fixes this problem and logs correct error message:
>
> Headset Voice gateway failed connect to XX:XX:XX:XX:XX:XX: Connection refused (111)
>
> Main problem was in ext_connect() function which called bt_io_get() for
> retrieving remote address (BT_IO_OPT_DEST) and if it failed then original
> error from connect() syscall was masked. Because it is not possible to
> retrieve BT_IO_OPT_DEST for unconnected socket, original destination
> address for error message is propagated via connect_add() function in btio.
>
> --
>
> Having correct error message in logs is important. Due to this mangled
> error message I was not able to easily debug why particular bluetooth
> headset sometimes connection with nonsense error that Transport endpoint
> was not connected.
Looks good, but lets have the btio changes as a separate patch.
> ---
> btio/btio.c | 19 ++++++++++++++-----
> src/profile.c | 5 +++--
> 2 files changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/btio/btio.c b/btio/btio.c
> index e7b4db16b..3ea73faea 100644
> --- a/btio/btio.c
> +++ b/btio/btio.c
> @@ -85,6 +85,7 @@ struct connect {
> BtIOConnect connect;
> gpointer user_data;
> GDestroyNotify destroy;
> + bdaddr_t dst;
> };
>
> struct accept {
> @@ -214,6 +215,7 @@ static gboolean connect_cb(GIOChannel *io, GIOCondition cond,
> GError *gerr = NULL;
> int err, sk_err, sock;
> socklen_t len = sizeof(sk_err);
> + char addr[18];
>
> /* If the user aborted this connect attempt */
> if ((cond & G_IO_NVAL) || check_nval(io))
> @@ -226,8 +228,11 @@ static gboolean connect_cb(GIOChannel *io, GIOCondition cond,
> else
> err = -sk_err;
>
> - if (err < 0)
> - ERROR_FAILED(&gerr, "connect error", -err);
> + if (err < 0) {
> + ba2str(&conn->dst, addr);
> + g_set_error(&gerr, BT_IO_ERROR, err,
> + "connect to %s: %s (%d)", addr, strerror(-err), -err);
> + }
>
> conn->connect(io, gerr, conn->user_data);
>
> @@ -286,7 +291,7 @@ static void server_add(GIOChannel *io, BtIOConnect connect,
> (GDestroyNotify) server_remove);
> }
>
> -static void connect_add(GIOChannel *io, BtIOConnect connect,
> +static void connect_add(GIOChannel *io, BtIOConnect connect, bdaddr_t dst,
> gpointer user_data, GDestroyNotify destroy)
> {
> struct connect *conn;
> @@ -296,6 +301,7 @@ static void connect_add(GIOChannel *io, BtIOConnect connect,
> conn->connect = connect;
> conn->user_data = user_data;
> conn->destroy = destroy;
> + conn->dst = dst;
>
> cond = G_IO_OUT | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
> g_io_add_watch_full(io, G_PRIORITY_DEFAULT, cond, connect_cb, conn,
> @@ -1671,6 +1677,7 @@ GIOChannel *bt_io_connect(BtIOConnect connect, gpointer user_data,
> struct set_opts opts;
> int err, sock;
> gboolean ret;
> + char addr[18];
>
> va_start(args, opt1);
> ret = parse_set_opts(&opts, gerr, opt1, args);
> @@ -1710,12 +1717,14 @@ GIOChannel *bt_io_connect(BtIOConnect connect, gpointer user_data,
> }
>
> if (err < 0) {
> - ERROR_FAILED(gerr, "connect", -err);
> + ba2str(&opts.dst, addr);
> + g_set_error(gerr, BT_IO_ERROR, err,
> + "connect to %s: %s (%d)", addr, strerror(-err), -err);
> g_io_channel_unref(io);
> return NULL;
> }
>
> - connect_add(io, connect, user_data, destroy);
> + connect_add(io, connect, opts.dst, user_data, destroy);
>
> return io;
> }
> diff --git a/src/profile.c b/src/profile.c
> index c2992e795..6961a107b 100644
> --- a/src/profile.c
> +++ b/src/profile.c
> @@ -1085,12 +1085,13 @@ static void ext_connect(GIOChannel *io, GError *err, gpointer user_data)
> if (!bt_io_get(io, &io_err,
> BT_IO_OPT_DEST, addr,
> BT_IO_OPT_INVALID)) {
> - error("Unable to get connect data for %s: %s", ext->name,
> - io_err->message);
> if (err) {
> + error("%s failed %s", ext->name, err->message);
> g_error_free(io_err);
> io_err = NULL;
> } else {
> + error("Unable to get connect data for %s: %s",
> + ext->name, io_err->message);
> err = io_err;
> }
> goto drop;
> --
> 2.20.1
>
--
Luiz Augusto von Dentz
