Warning fixes & clang-tidy #5876

In-line · 2024-05-04T15:13:25Z

Describe your PR, what does it fix/add?

Warning fixes, clang-tidy

Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)

Be careful while reading patch, some of the fixed parts of the code look suspicious.

Is it ready for merging, or does it need work?

Yes, if it passes review

vaxerski · 2024-05-04T15:16:35Z

I don't like these changes. Why?

In-line · 2024-05-04T15:23:44Z

@vaxerski Because Clang/GCC outputs warnings. Warnings are for a reason, they indicate some problem in the surrounding code. For most of the changes I just removed offending line, but I'm not sure it's correct.

vaxerski · 2024-05-04T15:38:41Z

not all automatic warnings matter, though. Furthermore, I don't see any gcc warnings when I compile (except that one in execAndGet but w/e)

In-line · 2024-05-04T19:29:23Z

Well, this are both GCC and Clang and Clang-tidy warnings, I tested with all of them, until there are no issues left.

If you want some parts to be removed, please feel free to tell me.

As this patch is fairly small in line count, I don't see how it will damage the project. On the other hand it removes spam from build logs and dead code.

In-line · 2024-05-04T19:34:26Z

As patches are separated in self-contained commits, please feel free to decide on what subset of the changes you are comfortable with. I'm not sure there is enough technical reasoning to drop all of them.

…ns but non-virtual destructor [-Wdelete-non-abstract-non-virtual-dtor]

…ommand-line-argument]

…not be used as a loop counter

…eference]

…-analyzer-deadcode.DeadStores]

In-line · 2024-05-05T19:21:09Z

@vaxerski If you don't have any intentions to merge this patches, please let me now, so I can stop keeping them up to date

vaxerski · 2024-05-06T14:27:07Z

I'll give you a review of which patches I am ok with and you can remove those not included, then we can merge

vaxerski

those with ok are to be left, the rest removed, and we can merge

vaxerski · 2024-05-06T14:27:22Z

CMakeLists.txt

+# currently provide well-defined behavior as an extension. 
+# However, GCC and Clang also warn about this use of offsetof,
+# which must be suppressed.
+set(WARNING_FLAGS -Wno-invalid-offsetof)


vaxerski · 2024-05-06T14:27:29Z

CMakeLists.txt

    endif()
+    target_sources(Hyprland PRIVATE protocols/${protoName}-protocol.c)
+    set_source_files_properties(protocols/${protoName}-protocol.c PROPERTIES COMPILE_FLAGS ${WARNING_FLAGS})


vaxerski · 2024-05-06T14:27:32Z

CMakeLists.txt

    endif()
+    target_sources(Hyprland PRIVATE protocols/${protoName}.cpp)
+    set_source_files_properties(protocols/${protoName}.cpp PROPERTIES COMPILE_FLAGS ${WARNING_FLAGS})


vaxerski · 2024-05-06T14:27:35Z

src/debug/HyprCtl.cpp

@@ -1754,7 +1754,8 @@ void CHyprCtl::startHyprCtlSocket() {

    std::string socketPath = g_pCompositor->m_szInstancePath + "/.socket.sock";

-    strcpy(SERVERADDRESS.sun_path, socketPath.c_str());
+    strncpy(SERVERADDRESS.sun_path, socketPath.c_str(), sizeof(SERVERADDRESS.sun_path) / sizeof(char));
+    SERVERADDRESS.sun_path[sizeof(SERVERADDRESS.sun_path) / sizeof(char) - 1] = '\0';


vaxerski · 2024-05-06T14:27:53Z

src/devices/IHID.hpp

@@ -16,6 +16,7 @@ enum eHIDCapabilityType : uint32_t {
 */
 class IHID {
  public:
+    virtual ~IHID() = default;


no: already in #5902

vaxerski · 2024-05-06T14:28:34Z

src/managers/input/InputManager.cpp

@@ -808,7 +808,7 @@ void CInputManager::setupKeyboard(SP<IKeyboard> keeb) {
        keeb.get());

    keeb->keyboardEvents.keymap.registerStaticListener(
-        [this](void* owner, std::any data) {
+        [](void* owner, std::any data) {


vaxerski · 2024-05-06T14:29:06Z

src/protocols/OutputManagement.cpp

@@ -287,7 +287,7 @@ COutputConfiguration::COutputConfiguration(SP<CZwlrOutputConfigurationV1> resour
        LOGM(LOG, "enableHead on {}. For now, doing nothing. Waiting for apply().", PMONITOR->szName);
    });

-    resource->setDisableHead([this](CZwlrOutputConfigurationV1* r, wl_resource* outputHead) {
+    resource->setDisableHead([](CZwlrOutputConfigurationV1* r, wl_resource* outputHead) {


vaxerski · 2024-05-06T14:29:15Z

src/protocols/PointerConstraints.hpp

@@ -34,7 +34,6 @@ class CPointerConstraint {
  private:
    SP<CZwpLockedPointerV1>         resourceL;
    SP<CZwpConfinedPointerV1>       resourceC;
-    wl_client*                      pClient = nullptr;


vaxerski · 2024-05-06T14:29:17Z

src/protocols/WaylandProtocol.cpp

@@ -28,5 +28,5 @@ IWaylandProtocol::IWaylandProtocol(const wl_interface* iface, const int& ver, co
 }

 IWaylandProtocol::~IWaylandProtocol() {
-    onDisplayDestroy();
+    IWaylandProtocol::onDisplayDestroy();


vaxerski · 2024-05-06T14:29:19Z

src/protocols/WaylandProtocol.hpp

@@ -14,7 +14,7 @@
 class IWaylandProtocol {
  public:
    IWaylandProtocol(const wl_interface* iface, const int& ver, const std::string& name);
-    ~IWaylandProtocol();
+    virtual ~IWaylandProtocol();


In-line added 13 commits May 5, 2024 23:18

Fix warning delete called on non-final class that has virtual functio…

b577375

…ns but non-virtual destructor [-Wdelete-non-abstract-non-virtual-dtor]

Fix warning lambda capture 'this' is not used [-Wunused-lambda-capture]

0b24bbb

Fix warning argument unused during compilation: '-no-pie' [-Wunused-c…

2180946

…ommand-line-argument]

Fix warning private field is not used [-Wunused-private-field]

944173e

Silence warning offset of on non-standard-layout type

9adcbe9

Add Clang-Tidy support

3447f3d

Fix clang-tidy: Variable 'i' with floating point type 'float' should …

64bbdc8

…not be used as a loop counter

Properly silence unused variables

4a539d1

Be explicit that called function is non-virtual

642ea81

Fix possible dereference of null pointer [clang-analyzer-core.NullDer…

06a2a1e

…eference]

Silence warning: Value stored to 'suppressEvent' is never read [clang…

dd0e932

…-analyzer-deadcode.DeadStores]

Fix unused assignment

94afd1b

Replace strcpy with strncpy

2674c43

In-line force-pushed the warning-fixes branch from 6f7e039 to 2674c43 Compare May 5, 2024 19:20

vaxerski reviewed May 6, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Warning fixes & clang-tidy #5876

Warning fixes & clang-tidy #5876

In-line commented May 4, 2024 •

edited

vaxerski commented May 4, 2024

In-line commented May 4, 2024 •

edited

vaxerski commented May 4, 2024

In-line commented May 4, 2024

In-line commented May 4, 2024 •

edited

In-line commented May 5, 2024

vaxerski commented May 6, 2024

vaxerski left a comment

vaxerski May 6, 2024

vaxerski May 6, 2024

vaxerski May 6, 2024

vaxerski May 6, 2024

vaxerski May 6, 2024

vaxerski May 6, 2024

vaxerski May 6, 2024

vaxerski May 6, 2024

vaxerski May 6, 2024

vaxerski May 6, 2024

Warning fixes & clang-tidy #5876

Are you sure you want to change the base?

Warning fixes & clang-tidy #5876

Conversation

In-line commented May 4, 2024 • edited

Describe your PR, what does it fix/add?

Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)

Is it ready for merging, or does it need work?

vaxerski commented May 4, 2024

In-line commented May 4, 2024 • edited

vaxerski commented May 4, 2024

In-line commented May 4, 2024

In-line commented May 4, 2024 • edited

In-line commented May 5, 2024

vaxerski commented May 6, 2024

vaxerski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

In-line commented May 4, 2024 •

edited

In-line commented May 4, 2024 •

edited

In-line commented May 4, 2024 •

edited