From: Edward Cree <ecree.xilinx@gmail.com>
To: Dan Carpenter <dan.carpenter@linaro.org>, oe-kbuild@lists.linux.dev
Cc: lkp@intel.com, oe-kbuild-all@lists.linux.dev,
linux-kernel@vger.kernel.org,
Simon Horman <simon.horman@corigine.com>
Subject: Re: drivers/net/ethernet/sfc/tc.c:450 efx_tc_flower_replace() warn: missing unwind goto?
Date: Tue, 30 May 2023 19:26:46 +0100 [thread overview]
Message-ID: <4eabd7b1-66b3-7b3a-cabd-d1876767c49c@gmail.com> (raw)
In-Reply-To: <cbbbf576-6788-4049-b1e8-a05862f62cc2@kili.mountain>
On 20/05/2023 09:32, Dan Carpenter wrote:
> b7f5e17b3bb961 Edward Cree 2023-03-27 432 if (efx_tc_match_is_encap(&match.mask)) {
> b7f5e17b3bb961 Edward Cree 2023-03-27 433 NL_SET_ERR_MSG_MOD(extack, "Ingress enc_key matches not supported");
> b7f5e17b3bb961 Edward Cree 2023-03-27 434 rc = -EOPNOTSUPP;
> b7f5e17b3bb961 Edward Cree 2023-03-27 435 goto release;
>
> This goto confuses Smatch. It could be converted to a direct return.
[...]
> d902e1a737d44e Edward Cree 2022-09-26 456 if (old) {
> d902e1a737d44e Edward Cree 2022-09-26 457 netif_dbg(efx, drv, efx->net_dev,
> d902e1a737d44e Edward Cree 2022-09-26 458 "Already offloaded rule (cookie %lx)\n", tc->cookie);
> d902e1a737d44e Edward Cree 2022-09-26 459 rc = -EEXIST;
> d902e1a737d44e Edward Cree 2022-09-26 460 NL_SET_ERR_MSG_MOD(extack, "Rule already offloaded");
> d902e1a737d44e Edward Cree 2022-09-26 461 goto release;
>
> It looks like this error path is problematic because it will remove the
> existing rule from the list. Better to just do:
>
> if (old) {
> netif_dbg(...);
> NL_SET_ERR_MSG_MOD(extack, "Rule already offloaded");
> kfree(rule);
> return -EEXIST;
> }
Agreed on both counts.
(Technically we could add a new goto label in the failure ladder to
just do the kfree and return, but this is probably clearer.)
Guess efx_tc_flower_replace_foreign() now has the latter problem
too? (And it _does_ want to use the failure ladder, because we've
got to release the encap match.)
Thanks for catching this. Patch for 'net' to follow shortly.
-ed
next prev parent reply other threads:[~2023-05-30 18:26 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-20 8:32 drivers/net/ethernet/sfc/tc.c:450 efx_tc_flower_replace() warn: missing unwind goto? Dan Carpenter
2023-05-30 18:26 ` Edward Cree [this message]
2023-05-31 8:11 ` Dan Carpenter
-- strict thread matches above, loose matches on Subject: below --
2023-05-19 23:53 kernel test robot
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=4eabd7b1-66b3-7b3a-cabd-d1876767c49c@gmail.com \
--to=ecree.xilinx@gmail.com \
--cc=dan.carpenter@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lkp@intel.com \
--cc=oe-kbuild-all@lists.linux.dev \
--cc=oe-kbuild@lists.linux.dev \
--cc=simon.horman@corigine.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.