Build errors at AssociationCommandConfiguration.cpp

[TheNetopyr]

Dear all,

Since a few weeks Open-zwave won' t build, it error's out with the following error on a clean git clone:

Building src/command_classes/Alarm.cpp
Building src/command_classes/ApplicationStatus.cpp
Building src/command_classes/AssociationCommandConfiguration.cpp
/home/user/open-zwave-read-only/cpp/src/command_classes/AssociationCommandConfiguration.cpp: In member function ‘virtual bool OpenZWave::Internal::CC::AssociationCommandConfiguration::HandleMsg(const uint8*, uint32, uint32)’:

/home/user/open-zwave-read-only/cpp/src/command_classes/AssociationCommandConfiguration.cpp:191:85: error: ‘this’ pointer is null [-Werror=nonnull]
191 | group->ClearCommands(nodeIdx);
| ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~
In file included from /home/user/open-zwave-read-only/cpp/src/Driver.h:36,
from /home/user/open-zwave-read-only/cpp/src/command_classes/CommandClass.h:36,
from /home/user/open-zwave-read-only/cpp/src/command_classes/AssociationCommandConfiguration.h:31,
from /home/user/open-zwave-read-only/cpp/src/command_classes/AssociationCommandConfiguration.cpp:29:
/home/user/open-zwave-read-only/cpp/src/Group.h:138:30: note: in a call to non-static member function ‘bool OpenZWave::Group::ClearCommands(uint8, uint8)’
138 | bool ClearCommands(uint8 const _nodeId, uint8 const _endPoint = 0x00);
| ^~~~~~~~~~~~~
/home/user/open-zwave-read-only/cpp/src/command_classes/AssociationCommandConfiguration.cpp:199:82: error: ‘this’ pointer is null [-Werror=nonnull]
199 | group->AddCommand(nodeIdx, length, start + 1);
| ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/user/open-zwave-read-only/cpp/src/Driver.h:36,
from /home/user/open-zwave-read-only/cpp/src/command_classes/CommandClass.h:36,
from /home/user/open-zwave-read-only/cpp/src/command_classes/AssociationCommandConfiguration.h:31,
from /home/user/open-zwave-read-only/cpp/src/command_classes/AssociationCommandConfiguration.cpp:29:
/home/user/open-zwave-read-only/cpp/src/Group.h:139:30: note: in a call to non-static member function ‘bool OpenZWave::Group::AddCommand(uint8, uint8, const uint8*, uint8)’
139 | bool AddCommand(uint8 const _nodeId, uint8 const _length, uint8 const* _data, uint8 const _endPoint = 0x00);
| ^~~~~~~~~~
cc1plus: all warnings being treated as errors
make[1]: *** [/home/user/open-zwave-read-only/cpp/build/support.mk:192: /home/user/open-zwave-read-only/.lib/AssociationCommandConfiguration.o] Error 1
make[1]: Leaving directory '/home/user/open-zwave-read-only/cpp/build'
make: *** [Makefile:23: all] Error 2

it is build with on a updated arch distribution with:
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-pc-linux-gnu/11.1.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: /build/gcc/src/gcc/configure --prefix=/usr --libdir=/usr/lib --libexecdir=/usr/lib --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=https://bugs.archlinux.org/ --enable-languages=c,c++,ada,fortran,go,lto,objc,obj-c++,d --with-isl --with-linker-hash-style=gnu --with-system-zlib --enable-__cxa_atexit --enable-cet=auto --enable-checking=release --enable-clocale=gnu --enable-default-pie --enable-default-ssp --enable-gnu-indirect-function --enable-gnu-unique-object --enable-install-libiberty --enable-linker-build-id --enable-lto --enable-multilib --enable-plugin --enable-shared --enable-threads=posix --disable-libssp --disable-libstdcxx-pch --disable-libunwind-exceptions --disable-werror gdc_include_dir=/usr/include/dlang/gdc
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 11.1.0 (GCC)

Is there anything I can do to fix this error?

Thanks in advance!

[waveform80]

The code in question doesn't make much sense: it tests whether group is NULL, then if it is, attempts to make several calls against the (definitely NULL) group object. The following patch reverses the sense of that test (!) and permits things to build:

--- a/cpp/src/command_classes/AssociationCommandConfiguration.cpp
+++ b/cpp/src/command_classes/AssociationCommandConfiguration.cpp
@@ -182,8 +182,7 @@
 
                                        if (Node* node = GetNodeUnsafe())
                                        {
-                                               Group* group = node->GetGroup(groupIdx);
-                                               if ( NULL == group)
+                                               if (Group* group = node->GetGroup(groupIdx))
                                                {
                                                        if (firstReports)
                                                        {

I'm guessing this is the original intent (?), but then again that code seems to have been unchanged since its introduction 11 years ago according to the git history, so I'm slightly surprised it's gone unnoticed for this long (unless it's incredibly rarely used? I'm afraid I'm not an openzwave user, but was looking into this build failure on Ubuntu's repos).

Happy to post that patch as a PR, but I'd like some confirmation from someone more familiar with the project that it does indeed make sense to reverse the sense of that NULL test, and there's not something more subtle that I'm missing!

[TheNetopyr]

Dear waveform80,

Thanks for this, although I am not exactly sure on the original intent, this pacth at least fixed the build issue and it continued to work again.

[babelouest]

Hello,

This issue breaks openzwave build in Debian unstable due to gcc-11: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=984279

The patch above also fixes the build with last stable version 1.6.1914, the tests are also OK with that patch.

I agree with @waveform80 's guess that the intent is not to test if ( NULL == group) but the opposite, but I wouldn't know how to validate this theory.

Would there be any side effect if we apply this change upstream?

[MagmaiKH]

Clearly the code was supposed to be if ( NULL != group)
diff patch below to fix
Note that no error is reported if the group comes back null.

diff --git a/cpp/src/command_classes/AssociationCommandConfiguration.cpp b/cpp/src/command_classes/AssociationCommandConfiguration.cpp
index 57644125..073edea8 100644
--- a/cpp/src/command_classes/AssociationCommandConfiguration.cpp
+++ b/cpp/src/command_classes/AssociationCommandConfiguration.cpp
@@ -183,7 +183,7 @@ namespace OpenZWave
 					if (Node* node = GetNodeUnsafe())
 					{
 						Group* group = node->GetGroup(groupIdx);
-						if ( NULL == group)
+						if ( NULL != group)
 						{
 							if (firstReports)
 							{

[slowriot]

Resolved by #2647