| Message ID | 20200605175942.719436-1-luiz.dentz@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [BlueZ] btio: Fix not translation mode to L2CAP mode | expand |
Hi, On Fri, Jun 5, 2020 at 10:59 AM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > When using L2CAP_OPTIONS legacy modes need to be used since they are > not compatible with BT_MODE. > --- > btio/btio.c | 24 ++++++++++++++++++++++-- > 1 file changed, 22 insertions(+), 2 deletions(-) > > diff --git a/btio/btio.c b/btio/btio.c > index 13c731062..844d6007f 100644 > --- a/btio/btio.c > +++ b/btio/btio.c > @@ -597,6 +597,20 @@ static gboolean get_key_size(int sock, int *size, GError **err) > return FALSE; > } > > +static uint8_t mode_l2mode(uint8_t mode) > +{ > + switch (mode) { > + case BT_IO_MODE_BASIC: > + return L2CAP_MODE_BASIC; > + case BT_IO_MODE_ERTM: > + return L2CAP_MODE_ERTM; > + case BT_IO_MODE_STREAMING: > + return L2CAP_MODE_STREAMING; > + default: > + return UINT8_MAX; > + } > +} > + > static gboolean set_l2opts(int sock, uint16_t imtu, uint16_t omtu, > uint8_t mode, GError **err) > { > @@ -614,8 +628,14 @@ static gboolean set_l2opts(int sock, uint16_t imtu, uint16_t omtu, > l2o.imtu = imtu; > if (omtu) > l2o.omtu = omtu; > - if (mode) > - l2o.mode = mode; > + > + if (mode) { > + l2o.mode = mode_l2mode(mode); > + if (l2o.mode == UINT8_MAX) { > + ERROR_FAILED(err, "Unsupported mode", errno); > + return FALSE; > + } > + } > > if (setsockopt(sock, SOL_L2CAP, L2CAP_OPTIONS, &l2o, sizeof(l2o)) < 0) { > ERROR_FAILED(err, "setsockopt(L2CAP_OPTIONS)", errno); > -- > 2.25.3 Pushed.
Hi Luiz, > Hi, > > On Fri, Jun 5, 2020 at 10:59 AM Luiz Augusto von Dentz > <luiz.dentz@gmail.com> wrote: > > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > > > When using L2CAP_OPTIONS legacy modes need to be used since they are > > not compatible with BT_MODE. > > --- > > btio/btio.c | 24 ++++++++++++++++++++++-- > > 1 file changed, 22 insertions(+), 2 deletions(-) > > > > diff --git a/btio/btio.c b/btio/btio.c > > index 13c731062..844d6007f 100644 > > --- a/btio/btio.c > > +++ b/btio/btio.c > > @@ -597,6 +597,20 @@ static gboolean get_key_size(int sock, int *size, GError **err) > > return FALSE; > > } > > > > +static uint8_t mode_l2mode(uint8_t mode) > > +{ > > + switch (mode) { > > + case BT_IO_MODE_BASIC: > > + return L2CAP_MODE_BASIC; > > + case BT_IO_MODE_ERTM: > > + return L2CAP_MODE_ERTM; > > + case BT_IO_MODE_STREAMING: > > + return L2CAP_MODE_STREAMING; > > + default: > > + return UINT8_MAX; > > + } > > +} > > + > > static gboolean set_l2opts(int sock, uint16_t imtu, uint16_t omtu, > > uint8_t mode, GError **err) > > { > > @@ -614,8 +628,14 @@ static gboolean set_l2opts(int sock, uint16_t imtu, uint16_t omtu, > > l2o.imtu = imtu; > > if (omtu) > > l2o.omtu = omtu; > > - if (mode) > > - l2o.mode = mode; > > + > > + if (mode) { > > + l2o.mode = mode_l2mode(mode); > > + if (l2o.mode == UINT8_MAX) { > > + ERROR_FAILED(err, "Unsupported mode", errno); > > + return FALSE; > > + } > > + } > > > > if (setsockopt(sock, SOL_L2CAP, L2CAP_OPTIONS, &l2o, sizeof(l2o)) < 0) { > > ERROR_FAILED(err, "setsockopt(L2CAP_OPTIONS)", errno); > > -- > > 2.25.3 > > Pushed. > > -- > Luiz Augusto von Dentz This patch seems to break avctp browsing. The creation of browsing_io in avctp_register already uses L2CAP_MODE_ERTM which is not in the switch-case hence results in "Unsupported mode". What is the desired way to fix this? Should all those calls use BT_IO_MODE_* instead? Not to mention the uses of these functions should be checked for their enum usage as well. - Marijn Suijten
Hi Marijn, On Mon, Jun 8, 2020 at 6:40 AM Marijn Suijten <marijns95@gmail.com> wrote: > > Hi Luiz, > > > Hi, > > > > On Fri, Jun 5, 2020 at 10:59 AM Luiz Augusto von Dentz > > <luiz.dentz@gmail.com> wrote: > > > > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > > > > > When using L2CAP_OPTIONS legacy modes need to be used since they are > > > not compatible with BT_MODE. > > > --- > > > btio/btio.c | 24 ++++++++++++++++++++++-- > > > 1 file changed, 22 insertions(+), 2 deletions(-) > > > > > > diff --git a/btio/btio.c b/btio/btio.c > > > index 13c731062..844d6007f 100644 > > > --- a/btio/btio.c > > > +++ b/btio/btio.c > > > @@ -597,6 +597,20 @@ static gboolean get_key_size(int sock, int *size, GError **err) > > > return FALSE; > > > } > > > > > > +static uint8_t mode_l2mode(uint8_t mode) > > > +{ > > > + switch (mode) { > > > + case BT_IO_MODE_BASIC: > > > + return L2CAP_MODE_BASIC; > > > + case BT_IO_MODE_ERTM: > > > + return L2CAP_MODE_ERTM; > > > + case BT_IO_MODE_STREAMING: > > > + return L2CAP_MODE_STREAMING; > > > + default: > > > + return UINT8_MAX; > > > + } > > > +} > > > + > > > static gboolean set_l2opts(int sock, uint16_t imtu, uint16_t omtu, > > > uint8_t mode, GError **err) > > > { > > > @@ -614,8 +628,14 @@ static gboolean set_l2opts(int sock, uint16_t imtu, uint16_t omtu, > > > l2o.imtu = imtu; > > > if (omtu) > > > l2o.omtu = omtu; > > > - if (mode) > > > - l2o.mode = mode; > > > + > > > + if (mode) { > > > + l2o.mode = mode_l2mode(mode); > > > + if (l2o.mode == UINT8_MAX) { > > > + ERROR_FAILED(err, "Unsupported mode", errno); > > > + return FALSE; > > > + } > > > + } > > > > > > if (setsockopt(sock, SOL_L2CAP, L2CAP_OPTIONS, &l2o, sizeof(l2o)) < 0) { > > > ERROR_FAILED(err, "setsockopt(L2CAP_OPTIONS)", errno); > > > -- > > > 2.25.3 > > > > Pushed. > > > > -- > > Luiz Augusto von Dentz > > This patch seems to break avctp browsing. The creation of browsing_io in > avctp_register already uses L2CAP_MODE_ERTM which is not in the > switch-case hence results in "Unsupported mode". What is the desired way > to fix this? Should all those calls use BT_IO_MODE_* instead? Not to > mention the uses of these functions should be checked for their enum > usage as well. I will fix it, bt_io API shall only operate with the its mode not L2CAP mode so it is a bug to pass them.
diff --git a/btio/btio.c b/btio/btio.c index 13c731062..844d6007f 100644 --- a/btio/btio.c +++ b/btio/btio.c @@ -597,6 +597,20 @@ static gboolean get_key_size(int sock, int *size, GError **err) return FALSE; } +static uint8_t mode_l2mode(uint8_t mode) +{ + switch (mode) { + case BT_IO_MODE_BASIC: + return L2CAP_MODE_BASIC; + case BT_IO_MODE_ERTM: + return L2CAP_MODE_ERTM; + case BT_IO_MODE_STREAMING: + return L2CAP_MODE_STREAMING; + default: + return UINT8_MAX; + } +} + static gboolean set_l2opts(int sock, uint16_t imtu, uint16_t omtu, uint8_t mode, GError **err) { @@ -614,8 +628,14 @@ static gboolean set_l2opts(int sock, uint16_t imtu, uint16_t omtu, l2o.imtu = imtu; if (omtu) l2o.omtu = omtu; - if (mode) - l2o.mode = mode; + + if (mode) { + l2o.mode = mode_l2mode(mode); + if (l2o.mode == UINT8_MAX) { + ERROR_FAILED(err, "Unsupported mode", errno); + return FALSE; + } + } if (setsockopt(sock, SOL_L2CAP, L2CAP_OPTIONS, &l2o, sizeof(l2o)) < 0) { ERROR_FAILED(err, "setsockopt(L2CAP_OPTIONS)", errno);
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> When using L2CAP_OPTIONS legacy modes need to be used since they are not compatible with BT_MODE. --- btio/btio.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-)