Hello Nithin,
[QUOTE]
I don't think we need two separate definitions. If ASSERT() is defined debug
only, then it would be a no-op in 'OVS_USE_ASSERTS'.
[/QUOTE]
The more complex assert-based macros would be useful if we would want to do a
more 'proper' cleanup in release mode, for a bug.
e.g.
if we have, say, within a function (I give some generic names)
pObj1 = LookupObject(container, cond1);
ASSERT(pObj1); //normally, pObj1 must be there, otherwise it's a bug
if (!pObj1) {
LOG_ERROR(err1)
return error_x;
}
pObj2 = LookupObject(container, cond2);
ASSERT(pObj2); //normally, pObj2 must be there, otherwise it's a bug
if (!pObj2) {
LOG_ERROR(err2)
return error_x;
}
...
A cleaner and more concise solution would be to do:
pObj1 = LookupObject(container, cond1);
OVS_CHECK_XXY(pObj1, err1);
pObj2 = LookupObject(container, cond2);
OVS_CHECK_XXY(pObj2, err2);
I think this patch was useful if the vport patch was added. Anyway, it's not
important now.
Sam
________________________________________
From: Nithin Raju [nit...@vmware.com]
Sent: Saturday, August 16, 2014 9:47 AM
To: Samuel Ghinet
Cc: dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH 09/15] datapath-windows: Add macros in Debug.h
Sam,
There are lot of assumptions here about how code should be structured and
written. eg. there's an implicit assumption that 'Cleanup' should be defined.
Let's table this for sometime till we get the current code starts working with
netlink.
If you can point me to some code what is using these macros, I might be able to
comment better.
> +//OVS_USE_ASSERTS is not #define-d on release mode
> +#if OVS_USE_ASSERTS
> +#define OVS_CHECK(x) ASSERT(x)
> +#define OVS_CHECK_OR(x, expr) { if (!(x)) { ASSERT(0); expr; } }
> +#define OVS_CHECK_BREAK(x) { if (!(x)) { ASSERT(0); break; } }
> +#define OVS_CHECK_RET(x, value) { if (!(x)) { ASSERT(0); return value; } }
> +#define OVS_CHECK_GC(x) { if (!(x)) { ASSERT(0); goto Cleanup; } }
> +#else
> +#define OVS_CHECK(x)
> +#define OVS_CHECK_OR(x, expr) { if (!(x)) expr; }
> +#define OVS_CHECK_BREAK(x) { if (!(x)) break; }
> +#define OVS_CHECK_RET(x, value) { if (!(x)) return value; }
> +#define OVS_CHECK_GC(x) { if (!(x)) goto Cleanup; }
> +#endif //OVS_USE_ASSERTS
I don't think we need two separate definitions. If ASSERT() is defined debug
only, then it would be a no-op in 'OVS_USE_ASSERTS'.
thanks,
Nithin
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
