zturner added inline comments.
================
Comment at: unittests/tools/lldb-server/tests/MessageObjects.cpp:61-63
+ string name;
+ thread_info->GetValueForKeyAsString("name", name);
+ string reason;
----------------
jmajors wrote:
> zturner wrote:
> > `StringRef name, reason;`
> There is no GetValueForKeyAsStringRef(). I need a std::string to populate.
As of r302875 this is no longer true.
================
Comment at: unittests/tools/lldb-server/tests/MessageObjects.cpp:83-88
+ if (endian == LITTLE) {
+ registers[register_id] = SwitchEndian(value_str);
+ }
+ else {
+ registers[register_id] = stoul(value_str, nullptr, 16);
+ }
----------------
jmajors wrote:
> zturner wrote:
> > jmajors wrote:
> > > zturner wrote:
> > > > This code block is unnecessary.
> > > >
> > > > ```
> > > > unsigned long X;
> > > > if (!value_str.getAsInteger(X))
> > > > return some error;
> > > > llvm::support::endian::write(®isters[register_id], X, endian);
> > > > ```
> > > >
> > > > By using llvm endianness functions you can just delete the
> > > > `SwitchEndian()` function entirely, as it's not needed.
> > > These endian functions don't work here. getAsInteger() assumes the number
> > > is in human reading order (i.e. big endian). So it's converting the
> > > little endian string as if it's big, then converting that little endian
> > > long to another little endian long, which does nothing.
> > > I need something that reverses the string first.
> > >
> > > What do you think about adding a version of StringRef::getAsInteger that
> > > accepts an endianness parameter?
> > Hmm.. What about this?
> >
> > ```
> > unsigned long X;
> > if (!value_str.getAsInteger(X))
> > return some error;
> > sys::swapByteOrder(X);
> > ```
> >
> > It shouldn't matter if you swap the bytes in the string before doing the
> > translation, or swap the bytes in the number after doing the translation
> > should it?
> It doesn't matter when I do it. The issue with the other functions was they
> were converting target to host, when both were little. For string parsing, it
> needs target to big to string, or target to string to big.
Maybe I'm just missing something, but if you've got the string "ABCDEF" which
is a little endian string, then it is supposed to represent the number
0x00EFCDAB or 15,715,755 (assuming a 4 byte number). If you parse it as big
endian, which is what `getAsInteger` does, you're going to end up with the
number 0xABCDEF00 which is 2,882,400,000, which is |AB|CD|EF|00| (on a big
endian machine) or |00|EF|CD|AB| (on a little endian machine). If you then
swap the bytes, you're going to end up with |00|EF|CD|AB| (on a big endian
machine) or |AB|CD|EF|00| on a little endian machine. In both cases, these
represent the number 15,715,755 as desired.
It's possible I'm just getting confused here, but I don't see why the above
code sample doesn't work.
https://reviews.llvm.org/D32930
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
