[Lldb-commits] [PATCH] D62499: Create a generic handler for Xfer packets

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2738
+  if (!memory_buffer_sp) {
+    if (xfer_object == "auxv") {
+// *BSD impls should be able to do this too.
----------------
aadsm wrote:
> xiaobai wrote:
> > labath wrote:
> > > Given that this function is going to grow, it would be good to split it in smaller chunks. Maybe move this code into something like `ErrorOr<xfer_map::iterator> ReadObject(StringRef object)`?
> > +1
> > 
> > You could have smaller methods like "ParseAuxvPacket" and "ReadObject"
> I'm actually trying to return an llvm::Expected so I can return a `createStringError` that will have a message and an error code (as far as I can see `ErrorOr`only allows to return an error code).
> However, I can't figure out how to get both the error code and error message from the `takeError()` function. I found `llvm::errorToErrorCode` and `llvm::toString` but they both require to pass ownership of the error to them so I can only use one of them.
> Is the only way to use the `handleErrors` function that will give me access to the underlying `ErrorInfo` where I can then call `convertToErrorCode` and `getMessage` on it? Sounds overly complicated for something that's probably simpler than this.
Yeah, using `Expected<T>` is a even better idea.

`handleErrors` is the right way to do this kind of thing. If you're using `Expected` then the best way to handle this situation would be to define a custom error type to mean "unimplemented". Then you could do something like:
```
Expected<T> t = getT();
if (!t) {
 ??? result;
  handleAllErrors(t.takeError(),
    [&] (UnimplementedError &e) { result = SendUnimplementedResponse(e.message()); },
   // TODO: We should have a SendErrorResponse version that takes a llvm::Error
    [&] (std::unique_ptr<ErrorInfo> e) { result = SendErrorResponse(Status(Error(std::move(e)));
  );
  return result;
}
do_stuff(*e);
```

Ideally this code wouldn't even live inside the packet handler function, but we would have a separate function for that, and packet handlers would just return `Expected<Packet>`. However, that's for another patch...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62499/new/

https://reviews.llvm.org/D62499