(CC Javier)
On Fri, Mar 10, 2023 at 2:52 PM Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote:
> >> > + for (i = 0; i < ARRAY_SIZE(dsi); i++) \
> >> > + mipi_dsi_dcs_write_seq(dsi[i], cmd, seq); \
> >>
> >> This ignores errors.
> > mipi_dsi_dcs_write_seq is also a macro, contains error checks in the body block.
>
> Ugh, I think it's pretty scary to hide control flow like return
> statements in macros like this.
The macros are written like this because:
#define mipi_dsi_generic_write_seq(dsi, seq...)
(...)
static const u8 d[] = { seq };
Array of bytes
ret = mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d));
So we can use use ARRAY_SIZE() in the macro and pass in any
arbitrary sequence, e.g.
mipi_dsi_generic_write_seq(dsi, 0xfb, 0x01);
Any function-esque definitions will (as in your example) require a
length to be passed in so it would become:
mipi_dsi_generic_write_seq(dsi, 0xfb, 0x01, 2);
And if you grep mipi_dsi_generic_write_seq | wc -l you find all the
245 opportunities to get that last len wrong and cause an out-of-bounds
bug.
I think this macro is the lesser evil for this reason, also it saves code
that you otherwise have to do manually, and one should never put
a person to do a machine's job.
Any suggestions to rewrite the macro using varargs welcome.
I think it is not very easy to do without the preprocessor.
Yours,
Linus Walleij
