Re: [PATCH] avrcp: Remove dead code

[Andrei Emeltchenko <Andrei.Emeltchenko.news@gmail.com>]

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@gmail.com> wrote:
> > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> >
> > 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