xiaobai added inline comments.
================
Comment at: source/Host/posix/HostThreadPosix.cpp:44
if (IsJoinable()) {
#ifndef __ANDROID__
#ifndef __FreeBSD__
----------------
labath wrote:
> xiaobai wrote:
> > aprantl wrote:
> > > What about:
> > > ```
> > > #ifdef __ANDROID__
> > > error.SetErrorString("HostThreadPosix::Cancel() not supported on
> > > Android");
> > > #else
> > > #ifdef __FreeBSD__
> > > int err = ::pthread_cancel(m_thread);
> > > error.SetError(err, eErrorTypePOSIX);
> > > #else
> > > llvm_unreachable("someone is calling HostThread::Cancel()");
> > > #endif
> > > #endif
> > > }
> > > return error;
> > > }
> > > ```
> > I agree with Adrian's suggestion, but I would add that you can remove one
> > of the `#endif` if you use `#elif defined(__FreeBSD__)` instead of an
> > `#else` + `#ifdef __FreeBSD__`.
> I think we can just unify the __ANDROID__ and __FreeBSD__ cases (turn both
> into unreachable). We only run lldb-server on android, and there we're mostly
> single-threaded, so there shouldn't be any thread cancelling going on
> anyway...
I don't think we can make the FreeBSD case unreachable (if I understand the
code correctly) since that's the one case when `::pthread_cancel` is actually
getting called.
If we can make the `__ANDROID__` case unreachable, this would very easily turn
into
```
#if defined(__FreeBSD__) || defined(__NetBSD__)
int err = ::pthread_cancel(m_thread);
error.SetError(err, eErrorTypePOSIX);
#else
llvm_unreachable("Someone is calling HostThreadPosix::Cancel()");
#endif
```
I'm somewhat unfamiliar with how exactly this code is used on android. If I
understand correctly, you're saying it's used in lldb-server and you meant that
lldb-server is mostly single threaded so this code shouldn't get run there?
https://reviews.llvm.org/D44056
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
