Hi Luiz,
On Mon, Jan 13, 2014 at 11:08:49AM +0200, Luiz Augusto von Dentz wrote:
> Hi Andrei,
>
> On Fri, Jan 10, 2014 at 4:56 PM, Andrei Emeltchenko
> <Andrei.Emeltchenko.news@xxxxxxxxx> wrote:
> > From: Andrei Emeltchenko <andrei.emeltchenko@xxxxxxxxx>
> >
> > session->target cannot be NULL since it is already checked 11 lines
> > above:
> > ...
> > if (session == NULL || session->target == NULL)
> > return -ENOTCONN;
> > ...
> > ---
> > profiles/audio/avrcp.c | 24 ++++--------------------
> > 1 file changed, 4 insertions(+), 20 deletions(-)
> >
> > diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
> > index 8d4309a..197959f 100644
> > --- a/profiles/audio/avrcp.c
> > +++ b/profiles/audio/avrcp.c
> > @@ -3722,30 +3722,14 @@ int avrcp_set_volume(struct btd_device *dev, uint8_t volume)
> >
> > DBG("volume=%u", volume);
> >
> > - if (session->target) {
> > - pdu->pdu_id = AVRCP_SET_ABSOLUTE_VOLUME;
> > - pdu->params[0] = volume;
> > - pdu->params_len = htons(1);
> > + pdu->pdu_id = AVRCP_SET_ABSOLUTE_VOLUME;
> > + pdu->params[0] = volume;
> > + pdu->params_len = htons(1);
> >
> > - return avctp_send_vendordep_req(session->conn,
> > + return avctp_send_vendordep_req(session->conn,
> > AVC_CTYPE_CONTROL, AVC_SUBUNIT_PANEL,
> > buf, sizeof(buf),
> > avrcp_handle_set_volume, session);
> > - } else if (session->registered_events &
> > - (1 << AVRCP_EVENT_VOLUME_CHANGED)) {
> > - uint8_t id = AVRCP_EVENT_VOLUME_CHANGED;
> > - pdu->pdu_id = AVRCP_REGISTER_NOTIFICATION;
> > - pdu->params[0] = AVRCP_EVENT_VOLUME_CHANGED;
> > - pdu->params[1] = volume;
> > - pdu->params_len = htons(2);
> > -
> > - return avctp_send_vendordep(session->conn,
> > - session->transaction_events[id],
> > - AVC_CTYPE_CHANGED, AVC_SUBUNIT_PANEL,
> > - buf, sizeof(buf));
> > - }
>
> The above code is necessary to notify volume changes when acting as
> controller (session->control is set), so your fix is not good either
> and we should depend on session->target on the first place because
> both session->control and session->target can be set.
So did you make a proper fix, since otherwise this is dead code?
Best regards
Andrei Emeltchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
