[Lldb-commits] [PATCH] D11574: Add size field to library load event

[Ilia K]

ki.stfu requested changes to this revision.
ki.stfu added a comment.
This revision now requires changes to proceed.

Why have you opened a new CL? Anyway, close/abandon the http://reviews.llvm.org/D9716 and see my inline comments.


================
Comment at: test/tools/lldb-mi/TestMiLibraryLoaded.py:30
@@ -29,2 +29,3 @@
         self.expect([
                 "=library-loaded,id=\"%s\",target-name=\"%s\",host-name=\"%s\",symbols-loaded=\"1\",symbols-path=\"%s\",loaded_addr=\"-\"" % (path, path, path, symbols_path),
+                "=library-loaded,id=\"%s\",target-name=\"%s\",host-name=\"%s\",symbols-loaded=\"0\",loaded_addr=\"-\",size=\"[0-9]+\"" % (path, path, path)
----------------
Add the `size` field here please.

================
Comment at: test/tools/lldb-mi/TestMiLibraryLoaded.py:31
@@ -30,3 +30,3 @@
                 "=library-loaded,id=\"%s\",target-name=\"%s\",host-name=\"%s\",symbols-loaded=\"1\",symbols-path=\"%s\",loaded_addr=\"-\"" % (path, path, path, symbols_path),
-                "=library-loaded,id=\"%s\",target-name=\"%s\",host-name=\"%s\",symbols-loaded=\"0\",loaded_addr=\"-\"" % (path, path, path)
+                "=library-loaded,id=\"%s\",target-name=\"%s\",host-name=\"%s\",symbols-loaded=\"0\",loaded_addr=\"-\",size=\"[0-9]+\"" % (path, path, path)
             ], exactly = True)
----------------
As I said in D9716, it will cause a false negative if `path` contains symbols that should be escaped (" or \ etc). I faced with the same issue in MiSyntaxTestCase. You should use analogue of CMIUtilString::AddSlashes here, or pass `exactly = True` option.

================
Comment at: tools/lldb-mi/MICmnLLDBDebuggerHandleEvents.cpp:731
@@ -730,1 +730,3 @@
         vwrMiOutOfBandRecord.Add(miValueResult6);
+        bOk = bOk && vwrMiOutOfBandRecord.Add(miValueResult6);
+        
----------------
ignore a return code here. It should cause a compiler error since r242911. Have you tried to compile it?

================
Comment at: tools/lldb-mi/MICmnLLDBDebuggerHandleEvents.cpp:735
@@ +734,3 @@
+        lldb::SBSection sbSection = sbAddress.GetSection();
+        const CMIUtilString strSize(CMIUtilString::Format("%PRIu64", sbSection.GetByteSize()));
+        const CMICmnMIValueConst miValueConst7(strSize);
----------------
Does it work for you? O_O

Should be:
```
const CMIUtilString strSize(CMIUtilString::Format("%" PRIu64, sbSection.GetByteSize()));
```

================
Comment at: tools/lldb-mi/MICmnLLDBDebuggerHandleEvents.cpp:738
@@ -731,1 +737,3 @@
+        const CMICmnMIValueResult miValueResult7("size", miValueConst7);
+        bOk = bOk && vwrMiOutOfBandRecord.Add(miValueResult7);
     }
----------------
ignore a return code


http://reviews.llvm.org/D11574