jmajors marked 5 inline comments as done.
jmajors added inline comments.
================
Comment at: unittests/tools/lldb-server/tests/MessageObjects.cpp:200
+
+ return std::stoul(big_endian, nullptr, 16);
+}
----------------
krytarowski wrote:
> Throws exceptions.
I didn't change this, because I'm expecting to remove the whole function.
================
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);
+ }
----------------
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.
================
Comment at: unittests/tools/lldb-server/tests/TestClient.cpp:76-80
+ for (size_t i = 0; i < inferior_args.size(); i++) {
+ if (i > 0) command << ',';
+ string hex_encoded = HexEncode(inferior_args[i]);
+ command << hex_encoded.size() << ',' << i << ',' << hex_encoded;
+ }
----------------
zturner wrote:
> `for (const auto &arg : inferior_args)`
I need an parameter index to pass to the command, so the classic for loop makes
more sense.
https://reviews.llvm.org/D32930
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
