All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: prabhakar.mahadev-lad.rj@bp.renesas.com
Cc: linux-renesas-soc@vger.kernel.org, linux-clk@vger.kernel.org
Subject: [bug report] clk: renesas: Add CPG core wrapper for RZ/G2L SoC
Date: Thu, 17 Jun 2021 15:02:19 +0300	[thread overview]
Message-ID: <YMs5yy57Jl7iRTo+@mwanda> (raw)

Hello Lad Prabhakar,

The patch ef3c613ccd68: "clk: renesas: Add CPG core wrapper for
RZ/G2L SoC" from Jun 9, 2021, leads to the following static checker
warning:

	drivers/clk/renesas/renesas-rzg2l-cpg.c:204 rzg2l_cpg_pll_clk_register()
	warn: passing devm_ allocated variable to kfree. 'pll_clk'

drivers/clk/renesas/renesas-rzg2l-cpg.c
   166  static struct clk * __init
   167  rzg2l_cpg_pll_clk_register(const struct cpg_core_clk *core,
   168                             struct clk **clks,
   169                             void __iomem *base,
   170                             struct rzg2l_cpg_priv *priv)
   171  {
   172          struct device *dev = priv->dev;
   173          const struct clk *parent;
   174          struct clk_init_data init;
   175          const char *parent_name;
   176          struct pll_clk *pll_clk;
   177          struct clk *clk;
   178  
   179          parent = clks[core->parent & 0xffff];
   180          if (IS_ERR(parent))
   181                  return ERR_CAST(parent);
   182  
   183          pll_clk = devm_kzalloc(dev, sizeof(*pll_clk), GFP_KERNEL);
   184          if (!pll_clk) {
   185                  clk = ERR_PTR(-ENOMEM);
   186                  return NULL;

Obviously, "return PTR_ERR(-ENOMEM); is intended.  But I looked at the
caller and it has a check for NULL as a kind of work around for this
bug.

When a function returns only NULL and valid pointers, the NULL is an
error.  But when a function returns *BOTH* NULL and error pointers the
NULL means that the feature is deliberately disabled by the user.  It's
not an error.  The caller should not print an error, but instead the
code needs to have NULL checks to avoid the NULL dereference.

For example, maybe the user has disabled blinking lights.

	lights = get_blinking_lights();

If get_blinking_lights() fails then we print an error message and return
to the user.  We don't try to continue.  The user can fix their problem
and try again.

But if the get_blinking_lights() returns NULL that means we have to add
NULL checks:

	if (lights)
		lights->blink();

   187          }
   188  
   189          parent_name = __clk_get_name(parent);
   190          init.name = core->name;
   191          init.ops = &rzg2l_cpg_pll_ops;
   192          init.flags = 0;
   193          init.parent_names = &parent_name;
   194          init.num_parents = 1;
   195  
   196          pll_clk->hw.init = &init;
   197          pll_clk->conf = core->conf;
   198          pll_clk->base = base;
   199          pll_clk->priv = priv;
   200          pll_clk->type = core->type;
   201  
   202          clk = clk_register(NULL, &pll_clk->hw);
   203          if (IS_ERR(clk))
   204                  kfree(pll_clk);

Delete this line.

   205  
   206          return clk;
   207  }

regards,
dan carpenter

             reply	other threads:[~2021-06-17 12:02 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-17 12:02 Dan Carpenter [this message]
  -- strict thread matches above, loose matches on Subject: below --
2021-06-17 13:37 [bug report] clk: renesas: Add CPG core wrapper for RZ/G2L SoC Dan Carpenter
2021-06-17 14:14 ` Prabhakar Mahadev Lad
2021-06-17 14:43   ` Dan Carpenter
2021-06-17 14:55     ` Prabhakar Mahadev Lad

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=YMs5yy57Jl7iRTo+@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=prabhakar.mahadev-lad.rj@bp.renesas.com \
    /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.