jingham added a comment.

This looks fine to me.  I'd give Greg a little time to weigh in, this is much 
more his code than mine.  But I don't see any problem with this, and thanks for 
the tests!



================
Comment at: source/Core/DumpDataExtractor.cpp:275-281
+      // Reject invalid item_byte_size.
+      if (item_byte_size > 8) {
+        s->Printf("error: unsupported byte size (%" PRIu64 ") for char format",
+                  (uint64_t)item_byte_size);
+        return offset;
+      }
+
----------------
Should this consume the weird input we couldn't print?  I actually don't have a 
good feel for which would be better.


================
Comment at: source/Utility/DataExtractor.cpp:566
                                   size_t byte_size) const {
-  switch (byte_size) {
-  case 1:
-    return GetU8(offset_ptr);
-    break;
-  case 2:
-    return GetU16(offset_ptr);
-    break;
-  case 4:
-    return GetU32(offset_ptr);
-    break;
-  default:
-    assert(false && "GetMaxU32 unhandled case!");
-    break;
-  }
-  return 0;
+  assert(byte_size <= 4 && "GetMaxU32 unhandled case!");
+  return GetMaxU64(offset_ptr, byte_size);
----------------
This is trivial, and you didn't change what was there, but this message makes 
it sound like this is just something we haven't gotten to yet.  It's really 
"You passed in an illegal byte size"...  Might be clearer if the message said 
that.


https://reviews.llvm.org/D38394



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to