[FFmpeg-devel] Array bug in aes.c

[Serge van den Boom]

On Fri Jan 18 13:25:23 CET 2008, Michael Niedermayer wrote:
> On Fri, Jan 18, 2008 at 06:12:16AM +0100, Serge van den Boom wrote:
> >      ((uint32_t *)
> >      enc_multbl)[sizeof(enc_multbl)/sizeof(enc_multbl[0][0]) - 1]
> > or
> >      *((uint32_t *) (&enc_multbl + 1) - 1)
>
> using of uint32_t is fragile as that would break if the type changes

I'm just using the type which you gave it. I would have typedef'ed the
element in the first place.

> > But if you mean "last element of the last element of enc_multbl", why
> > not write that?
> >      enc_multbl[sizeof(enc_multbl) / sizeof(enc_multbl[0]) - 1][
> >              sizeof(enc_multbl[0]) / sizeof(enc_multbl[0][0]) - 1]
> 
> bloated mess ...
> 
> [...]

That it is.
Still, given the choice between code which abuses the constructs which
exist to make code intuitive and more verifiable, while making the code
theoretically unportable (because undefined in the C standard),
and code which is portable and more intuitive, and where you don't have to
turn off potentially useful warnings, but which is a bit larger,
I would chose the latter.

But I would suggest to just use a few temporary vars (to be eliminated by the
compiler):
     size_t ySize = sizeof enc_multbl / sizeof enc_multbl[0];
     size_t xSize = sizeof enc_multbl[0] / sizeof enc_multbl[0][0];
     if (!enc_multbl[ySize - 1][xSize - 1]) ...
Clean, self-documenting code.

Or, as you know 'xSize' and 'ySize' from the start:
     #define MULTBL_XSIZE 256
     #ifdef CONFIG_SMALL
         #define MULTBL_YSIZE 1
     #else
         #define MULTBL_YSIZE 4
     #endif
     static uint32_t enc_multbl[MULTBL_YSIZE][MULTBL_XSIZE];
     static uint32_t dec_multbl[MULTBL_YSIZE][MULTBL_XSIZE];
     ...
     if (!enc_multbl[MULTBL_YSIZE - 1][MULTBL_XSIZE - 1]) ...

Plenty of ways to do things cleanly, I would say, without having to resort
to telling gcc to stop doing its job.


Regards,

Serge