Messages in this thread | | | Date | Fri, 20 Dec 2019 11:04:26 -0800 | From | Bjorn Andersson <> | Subject | Re: [PATCH] interconnect: Check for valid path in icc_set_bw() |
| |
On Fri 20 Dec 09:13 PST 2019, Georgi Djakov wrote:
> Use IS_ERR() to ensure that the path passed to icc_set_bw() is valid. > > Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org> > --- > drivers/interconnect/core.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c > index 63c164264b73..14a6f7ade44a 100644 > --- a/drivers/interconnect/core.c > +++ b/drivers/interconnect/core.c > @@ -498,6 +498,11 @@ int icc_set_bw(struct icc_path *path, u32 avg_bw, u32 peak_bw) > if (!path || !path->num_nodes) > return 0; > > + if (IS_ERR(path)) {
This is a sign of a logical error, and the print is likely to be ignored/lost in the noise. So I think the response should aid to help the developer hitting this to resolve the issue.
So I think this is more visible and more useful as:
if (WARN_ON(IS_ERR(path))) return -EINVAL;
PS. Doesn't path->num_nodes == 0 fall in this category as well? When would you have a path object with no nodes passed to this function?
Regards, Bjorn
> + pr_err("%s: invalid path=%ld\n", __func__, PTR_ERR(path)); > + return -EINVAL; > + } > + > mutex_lock(&icc_lock); > > old_avg = path->reqs[0].avg_bw;
| |