Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Build errors at AssociationCommandConfiguration.cpp #2613

Open
TheNetopyr opened this issue Jul 29, 2021 · 5 comments · May be fixed by #2641
Open

Build errors at AssociationCommandConfiguration.cpp #2613

TheNetopyr opened this issue Jul 29, 2021 · 5 comments · May be fixed by #2641

Comments

@TheNetopyr
Copy link

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
Copy link

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
Copy link
Author

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
Copy link

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?

@Ast-x64 Ast-x64 mentioned this issue Oct 22, 2021
Merged
@kiddigital kiddigital mentioned this issue Dec 27, 2021
Closed
@rjahanbakhshi rjahanbakhshi linked a pull request Aug 3, 2022 that will close this issue
Open
@MagmaiKH
Copy link

MagmaiKH commented Aug 7, 2022 •
edited
Loading

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)
 							{

@kiddigital kiddigital mentioned this issue Dec 4, 2022
Merged
@slowriot slowriot mentioned this issue Apr 18, 2023
Open
@slowriot
Copy link

Resolved by #2647

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

group pointer shouldn't be null to be dereferenced rjahanbakhshi/open-zwave
5 participants
@slowriot @waveform80 @MagmaiKH @babelouest @TheNetopyr