Thanks for the review! Pushed to master, Jarno
On Jul 18, 2014, at 10:40 PM, Ben Pfaff <b...@nicira.com> wrote: > On Fri, Jul 18, 2014 at 09:05:48PM -0700, Jarno Rajahalme wrote: >> As ovsrcu_get() looks like a function call, it is reasonable for the >> callers to expect that the arguments are evaluated only once. >> CONST_CAST expands its 'POINTER' argument multiple times, and the >> exact effect of this turned out to be compiler dependent. Fix this by >> expanding the macro argument before CONST_CAST, and removing >> unnecessary CONST_CASTs. >> >> VMware-BZ: #1287651 >> >> Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com> > > I'm really surprised that this has any effect. CONST_CAST is defined > as: > #define CONST_CAST(TYPE, POINTER) \ > ((void) sizeof ((int) ((POINTER) == (TYPE) (POINTER))), \ > (TYPE) (POINTER)) > and all the versions of the C standard say that "sizeof" does not > evaluate its argument, so this should have no effect on the generated > code; in fact that's the point of using the sizeof there. > > However, experimentation with GCC shows that while with GCC 4.6 and 4.7 > there is no difference or only trivial differences in the generated > code (across the whole source tree) for changing CONST_CAST to just > this: > #define CONST_CAST(TYPE, POINTER) ((TYPE) (POINTER)) > with GCC 4.1.2 in XenServer it makes a big difference. That probably > bears closer investigation on its own (is it a GCC bug? some > misunderstanding of ours?) by trying to reduce the differences to > minimal ones, but for now I guess we might as well just commit this. > > Acked-by: Ben Pfaff <b...@nicira.com> _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
