labath added inline comments.
================ Comment at: lldb/source/Host/common/Terminal.cpp:316-347 + bool parity_bit = true; + bool odd_bit = false; + bool stick_bit = false; + + switch (parity) { + case Parity::No: + parity_bit = false; ---------------- I am wondering if we can avoid the double conversion (enum->bools->flags). Would something like this be shorter/cleaner: ``` unsigned(?) GetParityFlagMask() { return PARENB | PARODD #ifdef CMSPAR | CMSPAR #endif ; } Expected<unsigned> GetParityFlags(Parity) // error if parity not supported SetParity(parity) { getParityFlags(parity); // and check result GetData(); // and check result data.m_termios.c_cflag &= ~GetParityFlagMask(); data.m_termios.c_cflag |= flags; SetData(); ``` ================ Comment at: lldb/unittests/Host/posix/TerminalTest.cpp:187-189 + llvm::Failed<llvm::ErrorInfoBase>(testing::Property( + &llvm::ErrorInfoBase::message, + "space/mark parity is not supported by the platform"))); ---------------- btw, we have `FailedWithMessage` for this these days. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111030/new/ https://reviews.llvm.org/D111030 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits