[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 17/17] hw/mips: Convert Malta "ifdef 0"-ed code to comment
From: |
Aleksandar Markovic |
Subject: |
Re: [PATCH v2 17/17] hw/mips: Convert Malta "ifdef 0"-ed code to comments |
Date: |
Fri, 15 May 2020 13:05:39 +0200 |
пет, 15. мај 2020. у 09:53 Markus Armbruster <address@hidden> је написао/ла:
>
> Aleksandar Markovic <address@hidden> writes:
>
> > The checkpatch complain about "#ifdef 0". Convert corresponding
> > dead code to comments. In future, these cases could be converted
> > to some no-nonsense logging/tracing.
> >
> > Signed-off-by: Aleksandar Markovic <address@hidden>
> > CC: Philippe Mathieu-Daudé <address@hidden>
> > ---
> > hw/mips/mips_malta.c | 20 ++++++++++++--------
> > 1 file changed, 12 insertions(+), 8 deletions(-)
> >
> > diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> > index e4c4de1b4e..f91fa34b06 100644
> > --- a/hw/mips/mips_malta.c
> > +++ b/hw/mips/mips_malta.c
> > @@ -427,10 +427,12 @@ static uint64_t malta_fpga_read(void *opaque, hwaddr
> > addr,
> > break;
> >
> > default:
> > -#if 0
> > - printf("malta_fpga_read: Bad register offset 0x" TARGET_FMT_lx
> > "\n",
> > - addr);
> > -#endif
> > +/*
> > + * Possible logging:
> > + *
> > + * printf("malta_fpga_read: Bad register offset 0x" TARGET_FMT_lx
> > "\n",
> > + * addr);
> > + */
> > break;
> > }
> > return val;
> > @@ -515,10 +517,12 @@ static void malta_fpga_write(void *opaque, hwaddr
> > addr,
> > break;
> >
> > default:
> > -#if 0
> > - printf("malta_fpga_write: Bad register offset 0x" TARGET_FMT_lx
> > "\n",
> > - addr);
> > -#endif
> > +/*
> > + * Possible logging:
> > + *
> > + * printf("malta_fpga_write: Bad register offset 0x" TARGET_FMT_lx
> > "\n",
> > + * addr);
> > + */
> > break;
> > }
> > }
>
> Please don't.
>
> Checkpatch warns "if this code is redundant consider removing it\n".
>
> If it is redundant, do remove it.
>
> If it is not redundant, do ignore checkpatch's warning, do not abuse
> comments to hide from checkpatch. We'd rather not have to code up a
> warning for that :)
>
> These two look like they want to be tracepoints.
>
Hi, Markus.
I understood your points. They make sense to me. In hindsight, in
general, we shouldn't try just to silence checkpatch warnings (or, for
that matter, compiler warnings as well), but try to resolve the root
cause, the underlying issue, of the warning. In this case, creating
tracepoints seems to be the right thing to do.
In hindsight too, this was my "quick and dirty" way of getting rid of
two checkpatch warnings.
Thanks for your remarks!
Aleksandar
P. S. The ultimate reason for this patch is that we plan renaming this
file in near future, and want to do it in "checkpatch-warning-free"
manner.
- [PATCH v2 07/17] target/mips: fpu: Demacro MSUB.<D|S|PS>, (continued)
- [PATCH v2 07/17] target/mips: fpu: Demacro MSUB.<D|S|PS>, Aleksandar Markovic, 2020/05/14
- [PATCH v2 09/17] target/mips: fpu: Demacro NMSUB.<D|S|PS>, Aleksandar Markovic, 2020/05/14
- [PATCH v2 06/17] target/mips: fpu: Demacro MADD.<D|S|PS>, Aleksandar Markovic, 2020/05/14
- [PATCH v2 10/17] target/mips: fpu: Remove now unused UNFUSED_FMA and FLOAT_FMA macros, Aleksandar Markovic, 2020/05/14
- [PATCH v2 15/17] target/mips: fpu: Name better paired-single variables, Aleksandar Markovic, 2020/05/14
- [PATCH v2 11/17] target/mips: fpu: Demacro CLASS.<D|S>, Aleksandar Markovic, 2020/05/14
- [PATCH v2 13/17] target/mips: fpu: Demacro RINT.<D|S>, Aleksandar Markovic, 2020/05/14
- [PATCH v2 16/17] target/mips: fpu: Refactor conversion from ieee to mips exception flags, Aleksandar Markovic, 2020/05/14
- [PATCH v2 17/17] hw/mips: Convert Malta "ifdef 0"-ed code to comments, Aleksandar Markovic, 2020/05/14
[PATCH v2 12/17] target/mips: fpu: Remove now unused FLOAT_CLASS macro, Aleksandar Markovic, 2020/05/14
[PATCH v2 08/17] target/mips: fpu: Demacro NMADD.<D|S|PS>, Aleksandar Markovic, 2020/05/14
[PATCH v2 14/17] target/mips: fpu: Remove now unused FLOAT_RINT macro, Aleksandar Markovic, 2020/05/14