I am not a fan of the VERIFY_SUCCESS macro. It is not prefixed with the OC_ 
which should be used for all iotivity macros to help prevent name collisions 
with macros from external projects.  Even with the prefix I think this is an 
internal macro that should not be exposed to the public APIs. I have seen some 
code recently that used the VERIFY_SUCCESS macro in sample code. The only 
reason it is building is because it is linking with internal headers. (not 100% 
sure about the non-public API statement)

We definitely should check where it is defined in multiple places and see if we 
can consolidate those definitions to a single definition.  If we cannot 
consolidate to one definition I would rename one of the usages.

Finally if nothing else works the #ifdef #undef // macro #endif option should 
be applied.

This is all opinion and I am open to other suggestions.

George Nash

-----Original Message-----
From: iotivity-dev-boun...@lists.iotivity.org 
[mailto:iotivity-dev-boun...@lists.iotivity.org] On Behalf Of Mats Wichmann
Sent: Tuesday, March 6, 2018 8:38 AM
To: iotivity-dev@lists.iotivity.org
Subject: Re: [dev] VERIFY_SUCCESS and other macros

On 03/06/2018 09:19 AM, Heldt-Sheller, Nathan wrote:
> Hi Devs,
> 
> I've noticed that we define macros (e.g. VERIFY_SUCCESS, and similar) in 
> several places, usually in the source file where it's being used.  This seems 
> like a maintenance burden and bug risk (if two implementations differ, it 
> might lead a developer to make a mistake).  It also can lead to collisions 
> when the macro appears in a header and source file.
> 
> One approach would be to wrap the macros in #ifndef //macro #endif blocks, to 
> resolve potential collisions, but again this seems error-prone, as a dev may 
> not know which version is actually being used when calling.
> 
> Another approach would be to wrap the macros in local source files with 
> #ifdef #undef // macro #endif blocks to ensure the locally-defined version is 
> used.
> 
> Another approach is to remove the implementations from source files entirely 
> and place in a shared "verifymacros.h" header file, but this would touch a 
> lot of files.
> 
> I'm sure there are others, too...
> 
> Any thoughts?

there's other duplication as well in the tree, which probably won't come as a 
surprise to people who've looked around a bit.


most of the macros seem to be in headers already, we should encourage that, 
although in some cases better choices could be made.  For the one you mention 
here's (slightly out of date) the stored information from my tags file, trimmed:

resource/csdk/security/include/srmutility.h, line 62 
cloud/samples/client/thin_light/thin_room_light.cpp, line 47 
plugins/samples/linux/IotivityandZigbeeServer.c, line 31 
resource/csdk/stack/samples/linux/SimpleClientServer/occlient.cpp, line 50 
resource/csdk/stack/samples/linux/SimpleClientServer/ocserver.cpp, line 48 
resource/csdk/stack/samples/tizen/SimpleClientServer/ocserver.cpp, line 39 
resource/csdk/stack/src/ocresource.c, line 72 
resource/csdk/stack/src/ocstack.c, line 180 
resource/csdk/stack/src/oickeepalive.c, line 46

This isn't great.

There are really two flavors of this one, the C form and the C++ form, with 
some variation across those.

I've thought about playing with some tools to flush out 
redundancies/inefficiencies, but nobody's paying me for working on this and the 
motivation just isn't always there. (sorry)



_______________________________________________
iotivity-dev mailing list
iotivity-dev@lists.iotivity.org
https://lists.iotivity.org/mailman/listinfo/iotivity-dev
_______________________________________________
iotivity-dev mailing list
iotivity-dev@lists.iotivity.org
https://lists.iotivity.org/mailman/listinfo/iotivity-dev

Reply via email to