teemperor requested changes to this revision.
teemperor added inline comments.
This revision now requires changes to proceed.
================
Comment at: lldb/tools/lldb-server/lldb-platform.cpp:289
fprintf(stderr, "failed to create acceptor: %s", error.AsCString());
- exit(socket_error);
+ return -1;
}
----------------
saschwartz wrote:
> teemperor wrote:
> > clayborg wrote:
> > > Should we return error.GetError() if it is non zero? IIRC it will be the
> > > actual errno. And best to not return -1, just return 1.
> > >
> > > ```
> > > uint32_t SBError::GetError() const;
> > > ```
> > If we force the caller to convert errno to an exit code, then we could also
> > just return the `Status error` itself (and then the caller can just return
> > 0/1 depending on success or error)? That seems more clear than returning
> > `errno` from a function with main signature (which makes it look like it
> > would return an exit code).
> Sounds fine to me - I went with `-1` because that was the original value for
> `socket_error`, but don't think anything should be conditioning on that.
>
> I'm pretty ambivalent to the `Status error` vs an error code directly myself,
> mainly because I don't know LLVM well enough to know what the convention
> might be. Will `error.GetError` always be nonzero if `error.Fail()` is true?
As said above, for this to work we need to have the caller still transform the
error code into a valid exit code. Status will give us any integer back (errno
or something else we made up), but if we return that from `main` then the exit
code will be set on UNIX to the lowest 8 bits of that value. So essentially
right now we implicitly do `exit_code = error % 256`. That only works if the
system agrees to never use a multiple of 256 as an error code and the same goes
for LLDB internally. And then there is also all the other weird stuff other
operating systems will do here.
I don't see any other error code in LLVM beside 0/1/-1 so let's just keep this
patch simple and return one of those from `main`. I don't think it matters a
lot what we return from this artificial main method, but if it's an `int` then
it looks like an exit code from the function signature and it should be a
reasonable exit code. So if we want to return an actual error code then it
should be wrapped in a `Status` to make it clear to the caller that they need
to convert it to an exit code.
> Will error.GetError always be nonzero if error.Fail() is true?
Yes, `GetError` returns non-zero on failure, but the clearer check is
`!error.Success()` (which does the same check under the hood).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108351/new/
https://reviews.llvm.org/D108351
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
