Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
MIME-Version: 1.0
References: <20231016154900.3094-1-iulia.tanasescu@nxp.com> <20231016154900.3094-3-iulia.tanasescu@nxp.com>
In-Reply-To: <20231016154900.3094-3-iulia.tanasescu@nxp.com>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Mon, 16 Oct 2023 10:04:59 -0700
Message-ID: <CABBYNZ+jgKfE7f7M1K_3WFrTPvfBRewb5u_q5SC8HQtmiRXzDA@mail.gmail.com>
Subject: Re: [PATCH BlueZ 2/4] shared/bass: Handle G_IO_HUP on io channels
To:     Iulia Tanasescu <iulia.tanasescu@nxp.com>
Cc:     linux-bluetooth@vger.kernel.org, claudia.rosu@nxp.com,
        mihai-octavian.urzica@nxp.com, silviu.barbulescu@nxp.com,
        vlad.pruteanu@nxp.com, andrei.istodorescu@nxp.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Iulia,

On Mon, Oct 16, 2023 at 8:49=E2=80=AFAM Iulia Tanasescu <iulia.tanasescu@nx=
p.com> wrote:
>
> This adds watches to handle closed sockets
>
> ---
>  src/shared/bass.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++
>  src/shared/bass.h |  2 ++
>  2 files changed, 61 insertions(+)
>
> diff --git a/src/shared/bass.c b/src/shared/bass.c
> index 0ee3187d1..78103d463 100644
> --- a/src/shared/bass.c
> +++ b/src/shared/bass.c
> @@ -655,6 +655,11 @@ static void connect_cb(GIOChannel *io, GError *gerr,
>                 g_io_channel_unref(bcast_src->listen_io);
>                 bcast_src->listen_io =3D NULL;
>
> +               if (bcast_src->listen_io_id > 0) {
> +                       g_source_remove(bcast_src->listen_io_id);
> +                       bcast_src->listen_io_id  =3D 0;
> +               }
> +
>                 /* Close pa sync io */
>                 if (bcast_src->pa_sync_io) {
>                         g_io_channel_shutdown(bcast_src->pa_sync_io,
> @@ -663,6 +668,11 @@ static void connect_cb(GIOChannel *io, GError *gerr,
>                         bcast_src->pa_sync_io =3D NULL;
>                 }
>
> +               if (bcast_src->pa_sync_io_id > 0) {
> +                       g_source_remove(bcast_src->pa_sync_io_id);
> +                       bcast_src->pa_sync_io_id  =3D 0;
> +               }
> +
>                 for (i =3D 0; i < bcast_src->num_subgroups; i++)
>                         bcast_src->subgroup_data[i].bis_sync =3D
>                                 BT_BASS_BIG_SYNC_FAILED_BITMASK;
> @@ -703,6 +713,18 @@ static bool bass_trigger_big_sync(struct bt_bcast_sr=
c *bcast_src)
>         return false;
>  }
>
> +static gboolean pa_sync_io_disconnect_cb(GIOChannel *io, GIOCondition co=
nd,
> +                       gpointer data)
> +{
> +       struct bt_bcast_src *bcast_src =3D data;
> +
> +       DBG(bcast_src->bass, "PA sync io has been disconnected");
> +
> +       bcast_src->pa_sync_io_id =3D 0;
> +       bcast_src->pa_sync_io =3D NULL;
> +
> +       return FALSE;
> +}
>
>  static void confirm_cb(GIOChannel *io, gpointer user_data)
>  {
> @@ -726,6 +748,15 @@ static void confirm_cb(GIOChannel *io, gpointer user=
_data)
>         bcast_src->pa_sync_io =3D io;
>         g_io_channel_ref(bcast_src->pa_sync_io);
>
> +       if (bcast_src->pa_sync_io_id > 0) {
> +               g_source_remove(bcast_src->pa_sync_io_id);
> +               bcast_src->pa_sync_io_id  =3D 0;
> +       }
> +
> +       bcast_src->pa_sync_io_id =3D g_io_add_watch(io, G_IO_ERR | G_IO_H=
UP |
> +                               G_IO_NVAL, (GIOFunc) pa_sync_io_disconnec=
t_cb,
> +                               bcast_src);
> +
>         len =3D sizeof(qos);
>         memset(&qos, 0, len);
>
> @@ -844,6 +875,19 @@ static bool bass_validate_add_src_params(uint8_t *va=
lue, size_t len)
>         return true;
>  }
>
> +static gboolean listen_io_disconnect_cb(GIOChannel *io, GIOCondition con=
d,
> +                       gpointer data)
> +{
> +       struct bt_bcast_src *bcast_src =3D data;
> +
> +       DBG(bcast_src->bass, "Listen io has been disconnected");
> +
> +       bcast_src->listen_io_id =3D 0;
> +       bcast_src->listen_io =3D NULL;
> +
> +       return FALSE;
> +}
> +
>  static void bass_handle_add_src_op(struct bt_bass *bass,
>                                         struct gatt_db_attribute *attrib,
>                                         uint8_t opcode,
> @@ -1023,6 +1067,11 @@ static void bass_handle_add_src_op(struct bt_bass =
*bass,
>                 bcast_src->listen_io =3D io;
>                 g_io_channel_ref(bcast_src->listen_io);
>
> +               bcast_src->listen_io_id =3D g_io_add_watch(io, G_IO_ERR |
> +                                       G_IO_HUP | G_IO_NVAL,
> +                                       (GIOFunc)listen_io_disconnect_cb,
> +                                       bcast_src);

IO handling shall probably be kept outside of shared so we could
emulate it with use of socketpairs and unit test it, see how it was
done for bap.

>                 if (num_bis > 0 && !bcast_src->bises)
>                         bcast_src->bises =3D queue_new();
>         } else {
> @@ -1318,11 +1367,21 @@ static void bass_bcast_src_free(void *data)
>                 g_io_channel_unref(bcast_src->listen_io);
>         }
>
> +       if (bcast_src->listen_io_id > 0) {
> +               g_source_remove(bcast_src->listen_io_id);
> +               bcast_src->listen_io_id  =3D 0;
> +       }
> +
>         if (bcast_src->pa_sync_io) {
>                 g_io_channel_shutdown(bcast_src->pa_sync_io, TRUE, NULL);
>                 g_io_channel_unref(bcast_src->pa_sync_io);
>         }
>
> +       if (bcast_src->pa_sync_io_id > 0) {
> +               g_source_remove(bcast_src->pa_sync_io_id);
> +               bcast_src->pa_sync_io_id  =3D 0;
> +       }
> +
>         queue_destroy(bcast_src->bises, bass_bis_unref);
>
>         free(bcast_src);
> diff --git a/src/shared/bass.h b/src/shared/bass.h
> index c4b5b76ba..bd3fe900b 100644
> --- a/src/shared/bass.h
> +++ b/src/shared/bass.h
> @@ -57,7 +57,9 @@ struct bt_bcast_src {
>         uint8_t num_subgroups;
>         struct bt_bass_subgroup_data *subgroup_data;
>         GIOChannel *listen_io;
> +       guint listen_io_id;
>         GIOChannel *pa_sync_io;
> +       guint pa_sync_io_id;
>         struct queue *bises;
>  };
>
> --
> 2.39.2
>


--=20
Luiz Augusto von Dentz