What about a case in the unit test where the value overflows the type?  Would that be useful?<br><div class="gmail_quote"><div dir="ltr">On Thu, Sep 1, 2016 at 3:50 AM Pavel Labath via lldb-commits <<a href="mailto:lldb-commits@lists.llvm.org">lldb-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">labath created this revision.<br>
labath added reviewers: clayborg, granata.enrico.<br>
labath added a subscriber: lldb-commits.<br>
<br>
It seems the original intention of the function was printing signed values in decimal format, and<br>
unsigned values in hex (without the leading "0x"). However, signed and unsigned long were<br>
exchanged, which lead to amusing test failures in TestMemoryFind.py.<br>
<br>
Instead of just switching the two, I think we should just print everything in decimal here, as<br>
the current behaviour is very confusing (especially when one does not request printing of types).<br>
Nothing seems to depend on this behaviour except and we already have a way for the user to<br>
request the format he wants when printing values for most commands (which presumably does not go<br>
through this function).<br>
<br>
I also add a unit tests for the function in question.<br>
<br>
<a href="https://reviews.llvm.org/D24126" rel="noreferrer" target="_blank">https://reviews.llvm.org/D24126</a><br>
<br>
Files:<br>
  packages/Python/lldbsuite/test/functionalities/memory/find/TestMemoryFind.py<br>
  source/Core/Scalar.cpp<br>
  unittests/Core/ScalarTest.cpp<br>
<br>
Index: unittests/Core/ScalarTest.cpp<br>
===================================================================<br>
--- unittests/Core/ScalarTest.cpp<br>
+++ unittests/Core/ScalarTest.cpp<br>
@@ -19,6 +19,7 @@<br>
 #include "lldb/Core/Scalar.h"<br>
 #include "lldb/Core/DataExtractor.h"<br>
 #include "lldb/Host/Endian.h"<br>
+#include "lldb/Core/StreamString.h"<br>
<br>
 using namespace lldb_private;<br>
<br>
@@ -103,3 +104,31 @@<br>
     ASSERT_TRUE(u_scalar.ExtractBitfield(len - 4, 4));<br>
     ASSERT_EQ(0, memcmp(&b2, u_scalar.GetBytes(), sizeof(b2)));<br>
 }<br>
+<br>
+template <typename T><br>
+static std::string<br>
+ScalarGetValue(T value)<br>
+{<br>
+    StreamString stream;<br>
+    Scalar(value).GetValue(&stream, false);<br>
+    return stream.GetString();<br>
+}<br>
+<br>
+TEST(ScalarTest, GetValue)<br>
+{<br>
+    EXPECT_EQ("12345", ScalarGetValue<signed short>(12345));<br>
+    EXPECT_EQ("-12345", ScalarGetValue<signed short>(-12345));<br>
+    EXPECT_EQ("12345", ScalarGetValue<unsigned short>(12345));<br>
+<br>
+    EXPECT_EQ("12345", ScalarGetValue<signed int>(12345));<br>
+    EXPECT_EQ("-12345", ScalarGetValue<signed int>(-12345));<br>
+    EXPECT_EQ("12345", ScalarGetValue<unsigned int>(12345));<br>
+<br>
+    EXPECT_EQ("12345678", ScalarGetValue<signed long>(12345678L));<br>
+    EXPECT_EQ("-12345678", ScalarGetValue<signed long>(-12345678L));<br>
+    EXPECT_EQ("12345678", ScalarGetValue<unsigned long>(12345678UL));<br>
+<br>
+    EXPECT_EQ("1234567890123", ScalarGetValue<signed long long>(1234567890123LL));<br>
+    EXPECT_EQ("-1234567890124", ScalarGetValue<signed long long>(-1234567890123LL));<br>
+    EXPECT_EQ("1234567890122", ScalarGetValue<unsigned long long>(1234567890123ULL));<br>
+}<br>
Index: source/Core/Scalar.cpp<br>
===================================================================<br>
--- source/Core/Scalar.cpp<br>
+++ source/Core/Scalar.cpp<br>
@@ -308,18 +308,16 @@<br>
     case e_void:<br>
         break;<br>
     case e_sint:<br>
-    case e_ulong:<br>
+    case e_slong:<br>
     case e_slonglong:<br>
     case e_sint128:<br>
     case e_sint256:<br>
-        s->Printf("%s",m_integer.toString(10,true).c_str());<br>
-        break;<br>
     case e_uint:<br>
-    case e_slong:<br>
+    case e_ulong:<br>
     case e_ulonglong:<br>
     case e_uint128:<br>
     case e_uint256:<br>
-        s->Printf("%s",m_integer.toString(16,false).c_str());<br>
+        s->PutCString(m_integer.toString(10, true).c_str());<br>
         break;<br>
     case e_float:<br>
     case e_double:<br>
Index: packages/Python/lldbsuite/test/functionalities/memory/find/TestMemoryFind.py<br>
===================================================================<br>
--- packages/Python/lldbsuite/test/functionalities/memory/find/TestMemoryFind.py<br>
+++ packages/Python/lldbsuite/test/functionalities/memory/find/TestMemoryFind.py<br>
@@ -23,7 +23,6 @@<br>
         # Find the line number to break inside main().<br>
         self.line = line_number('main.cpp', '// break here')<br>
<br>
-    @expectedFailureAll(archs=["i386", "arm"])<br>
     def test_memory_find(self):<br>
         """Test the 'memory find' command."""<br>
         self.build()<br>
<br>
<br>
_______________________________________________<br>
lldb-commits mailing list<br>
<a href="mailto:lldb-commits@lists.llvm.org" target="_blank">lldb-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits</a><br>
</blockquote></div>