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
