Skip to content

Commit 72ef113

Browse files
committed
[API] Have SBCommandReturnObject::GetOutput/Error return "" instead of nullptr
Summary: It seems this was an unintended side-effect of D26698. AFAICT, these functions did return an empty string before that patch, and the patch contained code which attempted to ensure that, but those efforts were negated by ConstString::AsCString, which by default returns a nullptr even for empty strings. This patch: - fixes the GetOutput/Error methods to really return empty strings - adds and explicit test for that - removes a workaround in lldbtest.py, which was masking this problem from our other tests Reviewers: jingham, clayborg Subscribers: zturner, lldb-commits Differential Revision: https://reviews.llvm.org/D65739 llvm-svn: 368806
1 parent 1427226 commit 72ef113

File tree

3 files changed

+21
-9
lines changed

3 files changed

+21
-9
lines changed

lldb/packages/Python/lldbsuite/test/lldbtest.py

-2
Original file line numberDiff line numberDiff line change
@@ -2318,8 +2318,6 @@ def expect(
23182318
with recording(self, trace) as sbuf:
23192319
print("looking at:", output, file=sbuf)
23202320

2321-
if output is None:
2322-
output = ""
23232321
# The heading says either "Expecting" or "Not expecting".
23242322
heading = "Expecting" if matching else "Not expecting"
23252323

lldb/packages/Python/lldbsuite/test/python_api/interpreter/TestCommandInterpreterAPI.py

+17
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
class CommandInterpreterAPICase(TestBase):
1313

1414
mydir = TestBase.compute_mydir(__file__)
15+
NO_DEBUG_INFO_TESTCASE = True
1516

1617
def setUp(self):
1718
# Call super's setUp().
@@ -72,3 +73,19 @@ def test_with_process_launch_api(self):
7273

7374
if self.TraceOn():
7475
lldbutil.print_stacktraces(process)
76+
77+
@add_test_categories(['pyapi'])
78+
def test_command_output(self):
79+
"""Test command output handling."""
80+
ci = self.dbg.GetCommandInterpreter()
81+
self.assertTrue(ci, VALID_COMMAND_INTERPRETER)
82+
83+
# Test that a command which produces no output returns "" instead of
84+
# None.
85+
res = lldb.SBCommandReturnObject()
86+
ci.HandleCommand("settings set use-color false", res)
87+
self.assertTrue(res.Succeeded())
88+
self.assertIsNotNone(res.GetOutput())
89+
self.assertEquals(res.GetOutput(), "")
90+
self.assertIsNotNone(res.GetError())
91+
self.assertEquals(res.GetError(), "")

lldb/source/API/SBCommandReturnObject.cpp

+4-7
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,8 @@ const char *SBCommandReturnObject::GetOutput() {
7272
LLDB_RECORD_METHOD_NO_ARGS(const char *, SBCommandReturnObject, GetOutput);
7373

7474
if (m_opaque_up) {
75-
llvm::StringRef output = m_opaque_up->GetOutputData();
76-
ConstString result(output.empty() ? llvm::StringRef("") : output);
77-
78-
return result.AsCString();
75+
ConstString output(m_opaque_up->GetOutputData());
76+
return output.AsCString(/*value_if_empty*/ "");
7977
}
8078

8179
return nullptr;
@@ -85,9 +83,8 @@ const char *SBCommandReturnObject::GetError() {
8583
LLDB_RECORD_METHOD_NO_ARGS(const char *, SBCommandReturnObject, GetError);
8684

8785
if (m_opaque_up) {
88-
llvm::StringRef output = m_opaque_up->GetErrorData();
89-
ConstString result(output.empty() ? llvm::StringRef("") : output);
90-
return result.AsCString();
86+
ConstString output(m_opaque_up->GetErrorData());
87+
return output.AsCString(/*value_if_empty*/ "");
9188
}
9289

9390
return nullptr;

0 commit comments

Comments
 (0)