Allowing compiling without DBUS.

[skilau]

There is a minor bug in CMakeLists.txt that always brings in common/dbus.c, even though later in the CMakeLists.txt file it is properly bounded by the check of "if(LIBSYSTEMD_FOUND)" (See line ~373 in CMakeLists.txt for it)

Because the line always brings in common/dbus.c, building without dbus isn't possible.
Removing the 1 line from the CMakeLists.txt fixes this issue.

[MathisMARION]

Hello, and thank you for pointing this out. Can you add back common/dbus.c in the D-Bus conditional sources?

[skilau]

Sorry about that. I actually thought that the line down in ~373 would bring it in. I should have tested it first.
I updated the PR to do:
$<$BOOL:${LIBSYSTEMD_FOUND}:common/dbus.c>

Instead, which does fix the compilation for both non-DBUS and DBUS builds.

[jerome-pouiller]

I am not sure a generator-expression makes sense here.

BTW, the project tries hard to keep atomic commit in the history. Can you squash this commit?

[skilau]

Hi! I did the squash.
I am not too familiar with CMake, so I wasn't exactly sure how to do the conditional include of the source file in the CMakeLists.txt file.

[jerome-pouiller]

diff --git i/CMakeLists.txt w/CMakeLists.txt
index 62f972f368..21a1f4e03b 100644
--- i/CMakeLists.txt
+++ w/CMakeLists.txt
@@ -270,7 +270,7 @@ if(LIBCAP_FOUND)
 endif()
 if(LIBSYSTEMD_FOUND)
     target_compile_definitions(libwsbrd PRIVATE HAVE_LIBSYSTEMD)
-    target_sources(libwsbrd PRIVATE app_wsbrd/app/dbus.c)
+    target_sources(libwsbrd PRIVATE common/dbus.c app_wsrd/app/dbus.c)
     if(AUTH_LEGACY)
         target_sources(libwsbrd PRIVATE app_wsbrd/app/dbus_auth_legacy.c)
     else()

[skilau]

Thanks! Fixed!

[skilau]

NOTE: Added a second minor patch on this PR as well.

I had to tag "wsbrd_dbus_vtable" as unused, or otherwise we get this error:

`[22/135] Building C object CMakeFiles/libwsbrd.dir/app_wsbrd/app/wsbr_mac.c.o
In file included from app_wsbrd/app/wsbr_mac.c:48:

app_wsbrd/app/dbus.h:24:36: warning: ‘wsbrd_dbus_vtable’ defined but not used [-Wunused-variable]
24 | static const struct sd_bus_vtable *wsbrd_dbus_vtable;
| ^~~~~~~~~~~~~~~~~`

[jerome-pouiller]

Weird, no warning should happen unless you compile with -Wunused-const-variable=2 (which is not the default). What compiler version are you using?

[MathisMARION]

GCC's -Wall does report this:

$ echo 'static int foo; int main() { }' | gcc -Wall -xc -
<stdin>:1:12: warning: ‘foo’ defined but not used [-Wunused-variable]

[MathisMARION]

This would remove the need for __attribute__((unused)) but I am not convinced that it is any better:

diff --git a/app_wsbrd/app/dbus.h b/app_wsbrd/app/dbus.h
index 3b5c4d6a18..3918c5c107 100644
--- a/app_wsbrd/app/dbus.h
+++ b/app_wsbrd/app/dbus.h
@@ -21,7 +21,7 @@ struct wsbr_ctxt;
 
 extern const struct sd_bus_vtable wsbrd_dbus_vtable[];
 #else
-static const struct sd_bus_vtable *wsbrd_dbus_vtable;
+#define wsbrd_dbus_vtable NULL
 #endif
 
 #endif

[jerome-pouiller]

The warning does not happen is the variable is declared in a header file:

$ cat foo.h
static const int foo;
$ cat foo.c
#include "foo.h"
int main() { }
$ gcc -Wall -c foo.c
$

[MathisMARION]

That's only partially true. It seems to also be a matter of constness:

$ echo 'static int foo;' > foo.h
$ gcc -Wall foo.c
In file included from foo.c:1:
foo.h:1:12: warning: ‘foo’ defined but not used [-Wunused-variable]
    1 | static int foo;
      |            ^~~

$ echo 'static const int foo;' > foo.h
$ gcc -Wall foo.c

And it gets a bit tricker with pointers:

$ echo 'static int *foo;' > foo.h
$ gcc -Wall foo.c
In file included from foo.c:1:
foo.h:1:13: warning: ‘foo’ defined but not used [-Wunused-variable]
    1 | static int *foo;
      |             ^~~

$ echo 'static const int *foo;' > foo.h
$ gcc -Wall foo.c
In file included from foo.c:1:
foo.h:1:19: warning: ‘foo’ defined but not used [-Wunused-variable]
    1 | static const int *foo;
      |                   ^~~

$ echo 'static int *const foo;' > foo.h
$ gcc -Wall foo.c

$ echo 'static const int *const foo;' > foo.h
$ gcc -Wall foo.c

So in our case the solution would be:

diff --git a/app_wsbrd/app/dbus.h b/app_wsbrd/app/dbus.h
index 3b5c4d6a18..16bf44dcf2 100644
--- a/app_wsbrd/app/dbus.h
+++ b/app_wsbrd/app/dbus.h
@@ -21,7 +21,7 @@ struct wsbr_ctxt;
 
 extern const struct sd_bus_vtable wsbrd_dbus_vtable[];
 #else
-static const struct sd_bus_vtable *wsbrd_dbus_vtable;
+static const struct sd_bus_vtable *const wsbrd_dbus_vtable;
 #endif
 
 #endif

[MathisMARION]

struct sd_bus_vtable is anomymous when we do not have systemd/sd-bus.h, so this would not compile. That is why we use a pointer instead.

[jerome-pouiller]

Arf, indeed.

[skilau]

Not sure if you still need my compiler version, but it is:
arm-linux-musleabi-gcc (GCC) 13.2.0

Yeah, I wasn't too happy using the "attribute ((unused)", but I couldn't find any particular better way to do it.

[jerome-pouiller]

As @MathisMARION said, static const struct sd_bus_vtable *const wsbrd_dbus_vtable; will do the job.

[skilau]

Awesome, thank you!
I changed it and squashed/pushed it.

[MathisMARION]

Applied onto our private development branch, thank you very much for your contribution! This should be released in version 2.3.1.