saschwartz marked 5 inline comments as done.
saschwartz added inline comments.
================
Comment at: lldb/tools/lldb-server/lldb-platform.cpp:289
fprintf(stderr, "failed to create acceptor: %s", error.AsCString());
- exit(socket_error);
+ return -1;
}
----------------
teemperor wrote:
> 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).
Sounds fine. That's a good argument about the implicit `% 256` operation which
I tend to agree would be an unexpected operation for the caller to have to deal
with. Sounds fine to stick with the typical semantics of a `main` function and
just `return 1` for all errors.
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
