Re: Warn on macros with flow control statements

[Julia Lawall <julia.lawall@inria.fr>]

> Based on the flow control I would do something like this.  It is a bit more
> verbose but is very clear what is happening.
>

Ira, what do you think of my suggestion of

err = err || qlge_fill_seg_(...);

It would avoid 40 lines of ifs, and would still avoid calling
qlge_fill_seg_ as soon as there is a failure.  On ther other hand, I don't
recall this being done elsewhere, so maybe all the ifs would be
preferable.

julia


> Ira
>
> 22:31:10 > git di
> diff --git a/drivers/staging/qlge/qlge_devlink.c b/drivers/staging/qlge/qlge_devlink.c
> index 0ab02d6d3817..043ed989f9a0 100644
> --- a/drivers/staging/qlge/qlge_devlink.c
> +++ b/drivers/staging/qlge/qlge_devlink.c
> @@ -39,15 +39,6 @@ static int qlge_fill_seg_(struct devlink_fmsg *fmsg,
>         return err;
>  }
>
> -#define FILL_SEG(seg_hdr, seg_regs)                                        \
> -       do {                                                                \
> -               err = qlge_fill_seg_(fmsg, &dump->seg_hdr, dump->seg_regs); \
> -               if (err) {                                                  \
> -                       kvfree(dump);                                       \
> -                       return err;                                         \
> -               }                                                           \
> -       } while (0)
> -
>  static int qlge_reporter_coredump(struct devlink_health_reporter *reporter,
>                                   struct devlink_fmsg *fmsg, void *priv_ctx,
>                                   struct netlink_ext_ack *extack)
> @@ -85,7 +76,12 @@ static int qlge_reporter_coredump(struct devlink_health_reporter *reporter,
>
>         qlge_soft_reset_mpi_risc(qdev);
>
> -       FILL_SEG(core_regs_seg_hdr, mpi_core_regs);
> +       err = qlge_fill_seg_(fmsg, &dump->core_regs_seg_hdr,
> +                            dump->mpi_core_regs);
> +       if (err)
> +               goto out;
> +
> +...  and all these others too...
>         FILL_SEG(test_logic_regs_seg_hdr, test_logic_regs);
>         FILL_SEG(rmii_regs_seg_hdr, rmii_regs);
>         FILL_SEG(fcmac1_regs_seg_hdr, fcmac1_regs);
> @@ -117,10 +113,8 @@ static int qlge_reporter_coredump(struct devlink_health_reporter *reporter,
>
>         err = qlge_fill_seg_(fmsg, &dump->misc_nic_seg_hdr,
>                              (u32 *)&dump->misc_nic_info);
> -       if (err) {
> -               kvfree(dump);
> -               return err;
> -       }
> +       if (err)
> +               goto out;
>
>         FILL_SEG(intr_states_seg_hdr, intr_states);
>         FILL_SEG(cam_entries_seg_hdr, cam_entries);
> @@ -139,6 +133,7 @@ static int qlge_reporter_coredump(struct devlink_health_reporter *reporter,
>         FILL_SEG(xfi2_hss_pll_hdr, serdes2_xfi_hss_pll);
>         FILL_SEG(sem_regs_seg_hdr, sem_regs);
>
> +out:
>         kvfree(dump);
>         return err;
>  }
>
>