On Thu, 2011-01-06 02:48:30 -0700, Tim Riker <[email protected]> wrote: > On 01/03/2011 12:57 PM, Jason Stevens wrote: > > wait, it should have been more like this > > > > > > > #define ML_SBR_TEST(r) if (((uint32)(r)) & 0xC000003u) != 0) > > > RSVD_OPND_FAULT > > > #define ML_PXBR_TEST(r) if (((((uint32)(r)) & 0x80000000u) == 0) || \ > > > (((uint32)(r)) & 0x40000003u) != 0)) > > > RSVD_OPND_FAULT > > > > into this: > > > > #define ML_SBR_TEST(r) if (((uint32)(r) & 0xC0000003u)!=0) RSVD_OPND_FAULT > > > > #define ML_PXBR_TEST(r) if (((((uint32)r) & 0x80000000u) == 0) || \ > > (((uint32)r) & 0x40000003u)!=0) RSVD_OPND_FAULT > You want the typecast to apply after any math that might be happening in > the expression r. > > You should include r in parentheses in the expansion. ie: not (uint32)r > ever, but instead (uint32)(r) so that r is evaluated first before > typecast is applied.
In terms of correctness, there are several things that could be made
better:
First of all, `r' is used twice here. The current uses seem sane, but
what, if `r' contained side effects (pre/post inc/dec etc.)? That
could probably be written as:
#define ML_PXBR_TEST(r) { \
uint32 ____t = (r); \
if ((____t & 0x80000000u) == 0) || \
____t & 0x40000003u) \
RSVD_OPND_FAULT; \
}
That would ensure `r' being evaluated only once. And it would
introduce a block around the if () ...; construct. Think of:
if (foo)
ML_PXBR_TEST (x);
else
do_something ();
Looks sane, but would work totally irrational with the original macro.
To accept a `;' after the closing brace of the block, maybe a do/while
should be added (as eg. the Linux kernel code does it):
#define ML_PXBR_TEST(r) do { \
uint32 ____t = (r); \
if ((____t & 0x80000000u) == 0) || \
____t & 0x40000003u) \
RSVD_OPND_FAULT; \
} while (0)
Comments?
MfG, JBG
--
Jan-Benedict Glaw [email protected] +49-172-7608481
Signature of: Zensur im Internet? Nein danke!
the second :
signature.asc
Description: Digital signature
_______________________________________________ Simh mailing list [email protected] http://mailman.trailing-edge.com/mailman/listinfo/simh
