From: Stephen Boyd <swboyd@chromium.org>
To: Murali Nalajala <mnalajal@codeaurora.org>,
gregkh@linuxfoundation.org, rafael@kernel.org
Cc: mnalajal@codeaurora.org, linux-arm-msm@vger.kernel.org,
linux-kernel@vger.kernel.org, bjorn.andersson@linaro.org
Subject: Re: [PATCH v1] base: soc: Handle custom soc information sysfs entries
Date: Thu, 03 Oct 2019 22:38:17 -0700 [thread overview]
Message-ID: <5d96daca.1c69fb81.fe5e4.e623@mx.google.com> (raw)
In-Reply-To: <1570146710-13503-1-git-send-email-mnalajal@codeaurora.org>
Quoting Murali Nalajala (2019-10-03 16:51:50)
> @@ -151,14 +156,16 @@ struct soc_device *soc_device_register(struct soc_device_attribute *soc_dev_attr
>
> ret = device_register(&soc_dev->dev);
> if (ret)
> - goto out3;
> + goto out4;
>
> return soc_dev;
>
> -out3:
> +out4:
> ida_simple_remove(&soc_ida, soc_dev->soc_dev_num);
> put_device(&soc_dev->dev);
> soc_dev = NULL;
> +out3:
> + kfree(soc_attr_groups);
This code is tricky. put_device(&soc_dev->dev) will call soc_release()
so we set soc_dev to NULL before calling kfree() on the error path. This
way we don't doubly free a pointer that the release function will take
care of. I wonder if the release function could free the ida as well,
and then we could just make the device_register() failure path call
put_device() and return ERR_PTR(ret) directly. Then the error path is
simpler because we can avoid changing two pointers to NULL to avoid the
double free twice. Or just inline the ida remove and put_device() call
in the if and then goto out1 to consolidate the error pointer
conversion.
> out2:
> kfree(soc_dev);
> out1:
next prev parent reply other threads:[~2019-10-04 5:38 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-03 23:51 [PATCH v1] base: soc: Handle custom soc information sysfs entries Murali Nalajala
2019-10-04 5:38 ` Stephen Boyd [this message]
2019-10-04 5:50 ` Bjorn Andersson
2019-10-04 6:17 ` Stephen Boyd
2019-10-04 7:05 ` Greg KH
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5d96daca.1c69fb81.fe5e4.e623@mx.google.com \
--to=swboyd@chromium.org \
--cc=bjorn.andersson@linaro.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mnalajal@codeaurora.org \
--cc=rafael@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.