Double cast of 2 enums with different size can lead to undefined behaviour in mozilla-2.0/media/libvpx/vp8/decoder/decodemv.c

[GoogleCodeExporter]

I am reading code from Firefox 4.0 release. In file 
mozilla-2.0/media/libvpx/vp8/decoder/decodemv.c I found a strange piece of 
code, which can lead to a problem:

In function vp8_read_mb_modes_mv I found:

=====
switch (bmi.mode = (B_PREDICTION_MODE) sub_mv_ref(bc, vp8_sub_mv_ref_prob2 
[mv_contz]))
{
   case NEW4X4:
...
=====

With a call to function sub_mv_ref:

=====
static MB_PREDICTION_MODE sub_mv_ref(vp8_reader *bc, const vp8_prob *p)
{
    const int i = vp8_treed_read(bc, vp8_sub_mv_ref_tree, p);

    return (MB_PREDICTION_MODE)i;
}
=====

With double cast of enumeration with different size, first the smaller one, 
than the bigger one.

=====
typedef enum
{
    DC_PRED,            /* average of above and left pixels */
    V_PRED,             /* vertical prediction */
    H_PRED,             /* horizontal prediction */
    TM_PRED,            /* Truemotion prediction */
    B_PRED,             /* block based prediction, each block has its own prediction mode */

    NEARESTMV,
    NEARMV,
    ZEROMV,
    NEWMV,
    SPLITMV,

    MB_MODE_COUNT
} MB_PREDICTION_MODE;

typedef enum
{
    B_DC_PRED,          /* average of above and left pixels */
    B_TM_PRED,

    B_VE_PRED,           /* vertical prediction */
    B_HE_PRED,           /* horizontal prediction */

    B_LD_PRED,
    B_RD_PRED,

    B_VR_PRED,
    B_VL_PRED,
    B_HD_PRED,
    B_HU_PRED,

    LEFT4X4,
    ABOVE4X4,
    ZERO4X4,
    NEW4X4,

    B_MODE_COUNT
} B_PREDICTION_MODE;
=====

IMHO this double cast can lead to undefined behaviour according to the C 
standard. The other question is: why should somebody make a double cast, what 
is the real intention?

Original issue reported on code.google.com by mct2...@gmail.com on 12 Jul 2011 at 7:13

[GoogleCodeExporter]

Original comment by iss...@webmproject.org on 12 Jul 2011 at 7:20

[GoogleCodeExporter]

It is fixed. Please check out commit: "Fix unnecessary casting of 
B_PREDICTION_MODE (issue 349)"(139577f9376e1607581a0152f97ba90b3af964f4).

Thanks.
Yunqing

Original comment by yunqingw...@google.com on 13 Jul 2011 at 8:37

• Changed state: Fixed

[GoogleCodeExporter]

I am sitting not at my own computer and do only have a browser and cannot look 
at the fix directly....

If "Fix unnecessary casting of B_PREDICTION_MODE" means to remove 
"(B_PREDICTION_MODE)" only, so the case "NEW4X4", which belongs to the larger 
enum can never be fulfilled. 

Please ignore my comment if my assumption is not correct.

Original comment by mct2...@gmail.com on 13 Jul 2011 at 9:05