On Wed, Jan 31, 2018 at 11:27 AM, Ben Pfaff <[email protected]> wrote:
> On Wed, Jan 31, 2018 at 11:03:04AM -0800, William Tu wrote:
>> The offsetof(struct ovs_router_entry, cr) should always be 0,
>> thus the else statement should never be reached.
>>
>> Signed-off-by: William Tu <[email protected]>
>> ---
>> lib/ovs-router.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/ovs-router.c b/lib/ovs-router.c
>> index cd2ab15fb003..bc8616858a84 100644
>> --- a/lib/ovs-router.c
>> +++ b/lib/ovs-router.c
>> @@ -71,7 +71,7 @@ ovs_router_entry_cast(const struct cls_rule *cr)
>> if (offsetof(struct ovs_router_entry, cr) == 0) {
>> return CONTAINER_OF(cr, struct ovs_router_entry, cr);
>> } else {
>> - return cr ? CONTAINER_OF(cr, struct ovs_router_entry, cr) : NULL;
>> + OVS_NOT_REACHED();
>> }
>> }
>
> This code seems odd, I would prefer to just write the general-purpose
> code and skip the microoptimizations, like this:
>
> struct ovs_router_entry *
> ovs_router_entry_cast(const struct cls_rule *cr)
> {
> return cr ? CONTAINER_OF(cr, struct ovs_router_entry, cr) : NULL;
> }
>
> GCC optimizes it properly anyhow, in all three versions:
>
> (gdb) disass ovs_router_entry_cast
> Dump of assembler code for function ovs_router_entry_cast:
> 0x00000e30 <+0>: mov 0x4(%esp),%eax
> 0x00000e34 <+4>: ret
> End of assembler dump.
OK thanks!
I will fix it in v2.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
