lkml.org 
[lkml]   [2013]   [Dec]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: bnx2x_sriov.c: Missing switch/case breaks?
From
Date
On Sat, 2013-12-14 at 06:16 +0000, Yuval Mintz wrote:
> > Hi Ariel.
> >
> > I wrote a little checkpatch script to look for missing
> > switch/case breaks.
> >
> > http://www.kernelhub.org/?msg=379933&p=2
> >
> > There are _many_ instances of case blocks in sriov.c
> > that could be missing breaks as they use fall-throughs.
> >
> > It would be good if these are actually intended to be
> > fall-throughs to add a /* fall-through */ comment between
> > each case block.
> >
> > For instance:
> >
> > static void bnx2x_vfop_qctor(struct bnx2x *bp, struct bnx2x_virtf *vf)
> > {
> > [...]
> > switch (state) {
> > case BNX2X_VFOP_QCTOR_INIT:
> >
> > /* has this queue already been opened? */
> > if (bnx2x_get_q_logical_state(bp, q_params->q_obj) ==
> > BNX2X_Q_LOGICAL_STATE_ACTIVE) {
> > DP(BNX2X_MSG_IOV,
> > "Entered qctor but queue was already up. Aborting
> > gracefully\n");
> > goto op_done;
> > }
> >
> > /* next state */
> > vfop->state = BNX2X_VFOP_QCTOR_SETUP;
> >
> > q_params->cmd = BNX2X_Q_CMD_INIT;
> > vfop->rc = bnx2x_queue_state_change(bp, q_params);
> >
> > bnx2x_vfop_finalize(vf, vfop->rc, VFOP_CONT);
>
> Hi Joe,

Hi Yuval.

> The `vfop' part of the code contains a lot of usage of the `bnx2x_vfop_finalize()',
> which either goto or return at the end of almost every case.
> "Normal" analysis tools/scripts fail to recognize them as valid case breaks.
>
> Adding `fallthrough' comments would make little sense, as this is not the real
> behavior; Perhaps we need some alternative comment? (something in the line
> of `macro case break')

No idea. It's certainly an ugly macro.

This does have a fallthrough path though when
(rc == 0 && next == VFOP_VERIFY_PEND) so
maybe there should be a break after most all
uses of this macro anyway. When next is
VFOP_VERIFY_PEND, then a "fall-through" comment
would be appropriate.

cheers, Joe

drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h:#define bnx2x_vfop_finalize(vf, rc, next) do { \
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h- if ((rc) < 0) \
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h- goto op_err; \
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h- else if ((rc) > 0) \
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h- goto op_pending; \
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h- else if ((next) == VFOP_DONE) \
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h- goto op_done; \
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h- else if ((next) == VFOP_VERIFY_PEND) \
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h- BNX2X_ERR("expected pending\n"); \
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h- else { \
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h- DP(BNX2X_MSG_IOV, "no ramrod. Scheduling\n"); \
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h- atomic_set(&vf->op_in_progress, 1); \
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h- queue_delayed_work(bnx2x_wq, &bp->sp_task, 0); \
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h- return; \
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h- } \
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h- } while (0)
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h-




\
 
 \ /
  Last update: 2013-12-14 08:21    [W:0.046 / U:0.176 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site