[lldb] [Host/Terminal] Fix warnings with termios disabled

[mgorny]

Summary

Thanks to Nico Weber for the suggestion.

[mgorny]

Create the error for !LLDB_ENABLE_TERMIOS via a helper function, as suggested by Nico Weber.

[thakis]

Thanks!

lldb/source/Host/common/Terminal.cpp

39–40

nit: should GetData() even exist if !LLDB_ENABLE_TERMIOS? Looks like it's only called #if LLDB_ENABLE_TERMIOS now. Maybe the whole method definition should be inside an ifdef.

If it should keep existing, either move the !FileDescriptorIsValid() check into the #if inside it (imho preferred), or move all the callers of it outside that #if. The current mix is a bit self-inconsistent.

[mgorny]

lldb/source/Host/common/Terminal.cpp

39–40

I think the goal was to avoid checking LLDB_ENABLE_TERMIOS in the header file, though I'm not sure if it was an explicit goal or one I've implicitly assumed.

I've move the check inside for the time being. We can always change it later.

lldb/source/Host/common/Terminal.cpp

 struct Terminal::Data {
 #if LLDB_ENABLE_TERMIOS
   struct termios m_termios; ///< Cached terminal state information.
 #endif
 };
 
 bool Terminal::IsATerminal() const { return m_fd >= 0 && ::isatty(m_fd); }
 
+#if !LLDB_ENABLE_TERMIOS
+static llvm::Error termiosMissingError() {
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                 "termios support missing in LLDB");
+}
+#endif
+
 llvm::Expected<Terminal::Data> Terminal::GetData() {
+#if LLDB_ENABLE_TERMIOS
   if (!FileDescriptorIsValid())
     return llvm::createStringError(llvm::inconvertibleErrorCode(),
                                    "invalid fd");
 
-#if LLDB_ENABLE_TERMIOS
   if (!IsATerminal())
     return llvm::createStringError(llvm::inconvertibleErrorCode(),
                                    "fd not a terminal");
 
   Data data;
   if (::tcgetattr(m_fd, &data.m_termios) != 0)
     return llvm::createStringError(
         std::error_code(errno, std::generic_category()),
         "unable to get teletype attributes");
   return data;
 #else // !LLDB_ENABLE_TERMIOS
-  return llvm::createStringError(llvm::inconvertibleErrorCode(),
-                                 "termios support missing in LLDB");
+  return termiosMissingError();
 #endif // LLDB_ENABLE_TERMIOS
 }
 
 llvm::Error Terminal::SetData(const Terminal::Data &data) {
 #if LLDB_ENABLE_TERMIOS
   assert(FileDescriptorIsValid());
   assert(IsATerminal());
 
   if (::tcsetattr(m_fd, TCSANOW, &data.m_termios) != 0)
     return llvm::createStringError(
         std::error_code(errno, std::generic_category()),
         "unable to set teletype attributes");
   return llvm::Error::success();
 #else // !LLDB_ENABLE_TERMIOS
-  llvm_unreachable("SetData() should not be called if !LLDB_ENABLE_TERMIOS");
+  return termiosMissingError();
 #endif // LLDB_ENABLE_TERMIOS
 }
 
 llvm::Error Terminal::SetEcho(bool enabled) {
+#if LLDB_ENABLE_TERMIOS
   llvm::Expected<Data> data = GetData();
   if (!data)
     return data.takeError();
 
-#if LLDB_ENABLE_TERMIOS
   struct termios &fd_termios = data->m_termios;
   fd_termios.c_lflag &= ~ECHO;
   if (enabled)
     fd_termios.c_lflag |= ECHO;
   return SetData(data.get());
+#else // !LLDB_ENABLE_TERMIOS
+  return termiosMissingError();
 #endif // LLDB_ENABLE_TERMIOS
 }
 
 llvm::Error Terminal::SetCanonical(bool enabled) {
+#if LLDB_ENABLE_TERMIOS
   llvm::Expected<Data> data = GetData();
   if (!data)
     return data.takeError();
 
-#if LLDB_ENABLE_TERMIOS
   struct termios &fd_termios = data->m_termios;
   fd_termios.c_lflag &= ~ICANON;
   if (enabled)
     fd_termios.c_lflag |= ICANON;
   return SetData(data.get());
+#else // !LLDB_ENABLE_TERMIOS
+  return termiosMissingError();
 #endif // LLDB_ENABLE_TERMIOS
 }
 
 llvm::Error Terminal::SetRaw() {
+#if LLDB_ENABLE_TERMIOS
   llvm::Expected<Data> data = GetData();
   if (!data)
     return data.takeError();
 
-#if LLDB_ENABLE_TERMIOS
   struct termios &fd_termios = data->m_termios;
   ::cfmakeraw(&fd_termios);
 
   // Make sure only one character is needed to return from a read
   // (cfmakeraw() doesn't do this on NetBSD)
   fd_termios.c_cc[VMIN] = 1;
   fd_termios.c_cc[VTIME] = 0;
 
   return SetData(data.get());
-#endif // #if LLDB_ENABLE_TERMIOS
+#else // !LLDB_ENABLE_TERMIOS
+  return termiosMissingError();
+#endif // LLDB_ENABLE_TERMIOS
 }
 
 #if LLDB_ENABLE_TERMIOS
 static llvm::Optional<speed_t> baudRateToConst(unsigned int baud_rate) {
   switch (baud_rate) {
 #if defined(B50)
   case 50:
     return B50;

 #endif
   default:
     return llvm::None;
   }
 }
 #endif
 
 llvm::Error Terminal::SetBaudRate(unsigned int baud_rate) {
+#if LLDB_ENABLE_TERMIOS
   llvm::Expected<Data> data = GetData();
   if (!data)
     return data.takeError();
 
-#if LLDB_ENABLE_TERMIOS
   struct termios &fd_termios = data->m_termios;
   llvm::Optional<speed_t> val = baudRateToConst(baud_rate);
   if (!val) // invalid value
     return llvm::createStringError(llvm::inconvertibleErrorCode(),
                                    "baud rate %d unsupported by the platform",
                                    baud_rate);
   if (::cfsetispeed(&fd_termios, val.getValue()) != 0)
     return llvm::createStringError(
         std::error_code(errno, std::generic_category()),
         "setting input baud rate failed");
   if (::cfsetospeed(&fd_termios, val.getValue()) != 0)
     return llvm::createStringError(
         std::error_code(errno, std::generic_category()),
         "setting output baud rate failed");
   return SetData(data.get());
-#endif // #if LLDB_ENABLE_TERMIOS
+#else // !LLDB_ENABLE_TERMIOS
+  return termiosMissingError();
+#endif // LLDB_ENABLE_TERMIOS
 }
 
 llvm::Error Terminal::SetStopBits(unsigned int stop_bits) {
+#if LLDB_ENABLE_TERMIOS
   llvm::Expected<Data> data = GetData();
   if (!data)
     return data.takeError();
 
-#if LLDB_ENABLE_TERMIOS
   struct termios &fd_termios = data->m_termios;
   switch (stop_bits) {
   case 1:
     fd_termios.c_cflag &= ~CSTOPB;
     break;
   case 2:
     fd_termios.c_cflag |= CSTOPB;
     break;
   default:
     return llvm::createStringError(
         llvm::inconvertibleErrorCode(),
         "invalid stop bit count: %d (must be 1 or 2)", stop_bits);
   }
   return SetData(data.get());
-#endif // #if LLDB_ENABLE_TERMIOS
+#else // !LLDB_ENABLE_TERMIOS
+  return termiosMissingError();
+#endif // LLDB_ENABLE_TERMIOS
 }
 
 llvm::Error Terminal::SetParity(Terminal::Parity parity) {
+#if LLDB_ENABLE_TERMIOS
   llvm::Expected<Data> data = GetData();
   if (!data)
     return data.takeError();
 
-#if LLDB_ENABLE_TERMIOS
   struct termios &fd_termios = data->m_termios;
   fd_termios.c_cflag &= ~(
 #if defined(CMSPAR)
       CMSPAR |
 #endif
       PARENB | PARODD);
 
   if (parity != Parity::No) {
     fd_termios.c_cflag |= PARENB;
     if (parity == Parity::Odd || parity == Parity::Mark)
       fd_termios.c_cflag |= PARODD;
     if (parity == Parity::Mark || parity == Parity::Space) {
 #if defined(CMSPAR)
       fd_termios.c_cflag |= CMSPAR;
 #else
       return llvm::createStringError(
           llvm::inconvertibleErrorCode(),
           "space/mark parity is not supported by the platform");
 #endif
     }
   }
   return SetData(data.get());
-#endif // #if LLDB_ENABLE_TERMIOS
+#else // !LLDB_ENABLE_TERMIOS
+  return termiosMissingError();
+#endif // LLDB_ENABLE_TERMIOS
 }
 
 llvm::Error Terminal::SetParityCheck(Terminal::ParityCheck parity_check) {
+#if LLDB_ENABLE_TERMIOS
   llvm::Expected<Data> data = GetData();
   if (!data)
     return data.takeError();
 
-#if LLDB_ENABLE_TERMIOS
   struct termios &fd_termios = data->m_termios;
   fd_termios.c_iflag &= ~(IGNPAR | PARMRK | INPCK);
 
   if (parity_check != ParityCheck::No) {
     fd_termios.c_iflag |= INPCK;
     if (parity_check == ParityCheck::Ignore)
       fd_termios.c_iflag |= IGNPAR;
     else if (parity_check == ParityCheck::Mark)
       fd_termios.c_iflag |= PARMRK;
   }
   return SetData(data.get());
-#endif // #if LLDB_ENABLE_TERMIOS
+#else // !LLDB_ENABLE_TERMIOS
+  return termiosMissingError();
+#endif // LLDB_ENABLE_TERMIOS
 }
 
 llvm::Error Terminal::SetHardwareFlowControl(bool enabled) {
+#if LLDB_ENABLE_TERMIOS
   llvm::Expected<Data> data = GetData();
   if (!data)
     return data.takeError();
 
-#if LLDB_ENABLE_TERMIOS
 #if defined(CRTSCTS)
   struct termios &fd_termios = data->m_termios;
   fd_termios.c_cflag &= ~CRTSCTS;
   if (enabled)
     fd_termios.c_cflag |= CRTSCTS;
   return SetData(data.get());
 #else  // !defined(CRTSCTS)
   if (enabled)
     return llvm::createStringError(
         llvm::inconvertibleErrorCode(),
         "hardware flow control is not supported by the platform");
   return llvm::Error::success();
 #endif // defined(CRTSCTS)
-#endif // #if LLDB_ENABLE_TERMIOS
+#else // !LLDB_ENABLE_TERMIOS
+  return termiosMissingError();
+#endif // LLDB_ENABLE_TERMIOS
 }
 
 TerminalState::TerminalState(Terminal term, bool save_process_group)
     : m_tty(term) {
   Save(term, save_process_group);
 }
 
 TerminalState::~TerminalState() { Restore(); }