Re: [PATCH v8 08/10] drivers: qcom: rpmh: allow requests to be sent asynchronously

[Raju P L S S S N]

Hi,

On 5/12/2018 1:46 AM, Doug Anderson wrote:
> Hi,
>
> On Wed, May 9, 2018 at 10:01 AM, Lina Iyer <ilina@xxxxxxxxxxxxxx> wrote:
> >   /**
> > @@ -137,6 +140,8 @@ void rpmh_tx_done(const struct tcs_request *msg, int r)
> >                  dev_err(rpm_msg->dev, "RPMH TX fail in msg addr=%#x, err=%d\n",
> >                          rpm_msg->msg.cmds[0].addr, r);
> >
> > +       kfree(rpm_msg->free);
> > +
>
> The way the code is written makes it seem like you could free memory
> _and_ have a completion but you can't.  Specifically:
>
> * The only plausible thing that "rpm_msg->free" could point to is "rpm_msg".
>
> * The complete(compl) would then be accessing freed memory.

As the completions are declared on stack, it will not access freed memory.

> I believe the kfree() should be at the end of the function.
> Personally I'd make it more obvious that this is just a boolean value
> and change to:
>
> if (rpm_msg->needs_free)
>    kgree(rpm_msg)
>
> ...then the reader of the code doesn't need to go figure out what
> you're trying to free.
>
>
> -Doug

Yes. Will change it this way in next patch set.

Thanks,
Raju