-
-
Notifications
You must be signed in to change notification settings - Fork 716
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.
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
Warning fixes & clang-tidy #5876
base: main
Are you sure you want to change the base?
Conversation
I don't like these changes. Why? |
@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. |
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) |
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. |
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
…-analyzer-deadcode.DeadStores]
@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 |
I'll give you a review of which patches I am ok with and you can remove those not included, then we can merge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
those with ok are to be left, the rest removed, and we can merge
# 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) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
endif() | ||
target_sources(Hyprland PRIVATE protocols/${protoName}-protocol.c) | ||
set_source_files_properties(protocols/${protoName}-protocol.c PROPERTIES COMPILE_FLAGS ${WARNING_FLAGS}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
endif() | ||
target_sources(Hyprland PRIVATE protocols/${protoName}.cpp) | ||
set_source_files_properties(protocols/${protoName}.cpp PROPERTIES COMPILE_FLAGS ${WARNING_FLAGS}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
@@ -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'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
@@ -16,6 +16,7 @@ enum eHIDCapabilityType : uint32_t { | |||
*/ | |||
class IHID { | |||
public: | |||
virtual ~IHID() = default; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no: already in #5902
@@ -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) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
@@ -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) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
@@ -34,7 +34,6 @@ class CPointerConstraint { | |||
private: | |||
SP<CZwpLockedPointerV1> resourceL; | |||
SP<CZwpConfinedPointerV1> resourceC; | |||
wl_client* pClient = nullptr; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
@@ -28,5 +28,5 @@ IWaylandProtocol::IWaylandProtocol(const wl_interface* iface, const int& ver, co | |||
} | |||
|
|||
IWaylandProtocol::~IWaylandProtocol() { | |||
onDisplayDestroy(); | |||
IWaylandProtocol::onDisplayDestroy(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
@@ -14,7 +14,7 @@ | |||
class IWaylandProtocol { | |||
public: | |||
IWaylandProtocol(const wl_interface* iface, const int& ver, const std::string& name); | |||
~IWaylandProtocol(); | |||
virtual ~IWaylandProtocol(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
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