On 01/23/2018 05:01 PM, Nash, George wrote: > See comments [geo] thanks, George.
> A. as described in (2), -Werror applies currently only to C code. > Should this flag be moved to CCFLAGS so it applies to both languages? > > > [geo] I thought the -Werror flag in CFLAGS was also being applied to C++. I > fixed warnings in both C and C++ so it should be applied to both. Since it > has not been applied to both it may take a while to fix up. I will look into > changing it to CCFLAGS. I just looked at the SCONS documentation CFLAGS: C > only, CCFLAGS: C and C++, CXXFLAGS: C++ only, CPPFLAGS anything using C > preprocessor C, C++ fortran, and assembly language source files. The > intention was to enforce the iotivity coding standards which are for C and > C++. I quote So to be clear, if you chased warnings from a build in making some patches, like I did, you saw all of them. -Werror in CFLAGS instead of CCFLAGS means the build would not have /failed/ if some of those were in C++ code, but we're still seeing all of them. > [geo] "Code must compile with no warnings at the standard warning level > without flags forcing any warnings off. Exceptions are allowed only for > warnings that occur within any external SDKs used or warnings that cannot be > eliminated without using pragmatics (e.g. for nested STL templates, certain > compilers will warn that names have been truncated). For these cases, #pragma > directives may be used to eliminate the warning, but only for the module in > which the warning occurs. [C/C++]" > (https://wiki.iotivity.org/iotivity_c_coding_standards) > > B. how to handle the tinycbor warnings, since the discussion in IOT-2539 does > not apply this this case? > Since the tinycbor files are only built in to ocsrm library I chose to ignore > those warnings when building the library. See > https://gerrit.iotivity.org/gerrit/#/c/23965/ [still in review] I felt this > made since. Its limited to a small collection of files that the extra > warnings are ignored on. If the warnings are fixed on tinycbor ignore flags > can be removed. Yes, as noted in a separate reply, I think this approach is about the best we are going to do. I think the warnings are already fixed in tinycbor, there's just a delay in moving forward (and in my opinion, 1.3-rel should continue to use the version it was QA'd with, so this workaround might want to stick around in that branch). > [geo] Any warning can be ignored if needed but the developer that turns off a > build warning should try and limit it to the smallest amount of files as > possible and they should justify their reason for turning it off. That's all good; it's a little awkward when the files to turn off warnings are a subset of a larger build since scons isn't really good at letting you apply flags to individual files. > E. Deprecated usage in unittests, based on our own deprecation of two > functions b64Encode and b64Decode (using the new OC_DEPRECATED stuff). > In fact we deprecated the whole header base64.h. So this deprecation > is causing build fails... was that actually intended? Or is the fail part > premature? > > > [geo] the only thing that should continue to use the b64Encode/b64Decode > inside IoTivity are the unit tests. And the deprecated warning should be > ignored for that code. If it is not being ignored due to changes in the > compiler we should update the scons to ignore that build failure. > > resource/csdk/security/unittest/base64tests.cpp:103:79: warning: > 'B64Result b64Encode(const uint8_t*, size_t, char*, size_t, size_t*)' is > deprecated: Please use mbedtls implementation [-Wdeprecated-declarations] > > Since the unit test is testing the behavior of this deprecated code, this > creates an interesting dilemma: as long as the code still exists, it should > have tests, no? but the test will cause build fails under the current > scenario. > > [geo] Yes, as long as the code still exists unit tests should still be run. So I'd propose for this one file adding a comment to this effect, and them adding: #pragma GCC diagnostic ignored "-Wdeprecated-declarations" That will work for gcc; will there be a problem on rather different compilers like Windows when they hit the new deprecation stuff? _______________________________________________ iotivity-dev mailing list [email protected] https://lists.iotivity.org/mailman/listinfo/iotivity-dev
