Skip to content

[lldb] Fix redundant condition in Target.cpp #91882

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

Merged
merged 2 commits into from
May 14, 2024

Conversation

aabhinavg
Copy link
Member

This commit addresses issue #87244, where a redundant condition was found in the Target.cpp file. Static analyzer cppcheck flagged the issue in the Target.cpp file
fix #87244

This commit addresses issue llvm#87244, where a redundant condition was found in the Target.cpp file.
Static analyzer cppcheck flagged the issue in the Target.cpp file
@aabhinavg aabhinavg requested a review from JDevlieghere as a code owner May 12, 2024 07:17
@llvmbot llvmbot added the lldb label May 12, 2024
@llvmbot
Copy link
Member

llvmbot commented May 12, 2024

@llvm/pr-subscribers-lldb

Author: None (aabhinavg)

Changes

This commit addresses issue #87244, where a redundant condition was found in the Target.cpp file. Static analyzer cppcheck flagged the issue in the Target.cpp file
fix #87244


Full diff: https://github.com/llvm/llvm-project/pull/91882.diff

1 Files Affected:

  • (modified) lldb/source/Target/Target.cpp (+8-6)
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 82f3040e539a3..fe87728a33dc8 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -841,12 +841,14 @@ static bool CheckIfWatchpointsSupported(Target *target, Status &error) {
   if (!num_supported_hardware_watchpoints)
     return true;
 
-  if (num_supported_hardware_watchpoints == 0) {
-    error.SetErrorStringWithFormat(
-        "Target supports (%u) hardware watchpoint slots.\n",
-        *num_supported_hardware_watchpoints);
-    return false;
-  }
+  // If num_supported_hardware_watchpoints is zero, set an 
+  //error message and return false.
+
+  error.SetErrorStringWithFormat(
+      "Target supports (%u) hardware watchpoint slots.\n",
+      *num_supported_hardware_watchpoints);
+  return false;
+  
   return true;
 }
 

@aabhinavg aabhinavg self-assigned this May 12, 2024
Copy link

github-actions bot commented May 12, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Collaborator

@DavidSpickett DavidSpickett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this function got a bit confused when we changed the return type to an optional, thanks for taking a look at it.

This reverts commit 9b41609.

The CheckIfWatchpointsSupported function is refactored to maintain the intended behavior as described below:

1. If we can't detect hardware watchpoints, assume they are supported.
2. If we can detect hardware watchpoints and there are more than 0, return true.
3. If we can detect hardware watchpoints but there are 0 of them, set an error message and return false.

fix llvm#87244
@aabhinavg aabhinavg force-pushed the fix-watchpoint-issue branch from 9c543a6 to 421b4ee Compare May 13, 2024 16:01
Copy link
Collaborator

@DavidSpickett DavidSpickett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@DavidSpickett DavidSpickett merged commit c285297 into llvm:main May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lldb/source/Target/Target.cpp:844: function only ever returns true ?
4 participants