wallace marked 12 inline comments as done.
wallace added inline comments.
================
Comment at: lldb/source/Target/Trace.cpp:109
+ StringRef load_address_str;
+ if (!module->GetValueForKeyAsString("loadAddress", load_address_str))
+ return SetMissingFieldError(error, "loadAddress", "string", *module);
----------------
wallace wrote:
> clayborg wrote:
> > clayborg wrote:
> > > does JSON typically use camel case? Should this be "load-address"?
> > Should load address be encoded as an integer to begin with? I know it is
> > less readable as an integer since we can't use hex numbers It would
> > simplify the logic here. If we want to use strings, we should make a helper
> > function that decodes an address from a key's value in a dictionary so we
> > can re-use elsewhere.
> yes, it's camel case typically
The way this is implemented is that it's expecting a string number in any
radix. You can pass "123123" or "0xFFAABBFF" for example. You cannot pass it
directly as a simple number because JSON doesn't support 64-bit integers
================
Comment at: lldb/source/Target/Trace.cpp:109-110
+ StringRef load_address_str;
+ if (!module->GetValueForKeyAsString("loadAddress", load_address_str))
+ return SetMissingFieldError(error, "loadAddress", "string", *module);
+ addr_t load_address;
----------------
clayborg wrote:
> clayborg wrote:
> > does JSON typically use camel case? Should this be "load-address"?
> Should load address be encoded as an integer to begin with? I know it is less
> readable as an integer since we can't use hex numbers It would simplify the
> logic here. If we want to use strings, we should make a helper function that
> decodes an address from a key's value in a dictionary so we can re-use
> elsewhere.
yes, it's camel case typically
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85705/new/
https://reviews.llvm.org/D85705
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
