[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

32

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

32

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); }
 
 llvm::Expected<Terminal::Data> Terminal::GetData() {
   if (!FileDescriptorIsValid())
     return llvm::createStringError(llvm::inconvertibleErrorCode(),
                                    "invalid fd");
 
 #if LLDB_ENABLE_TERMIOS
   if (!IsATerminal())
     return llvm::createStringError(llvm::inconvertibleErrorCode(),
                                    "fd not a terminal");
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
+  llvm_unreachable("GetData() should have returned an error");
 #endif // LLDB_ENABLE_TERMIOS
 }
 
 llvm::Error Terminal::SetCanonical(bool enabled) {
   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
+  llvm_unreachable("GetData() should have returned an error");
 #endif // LLDB_ENABLE_TERMIOS
 }
 
 llvm::Error Terminal::SetRaw() {
   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());
+#else
+  llvm_unreachable("GetData() should have returned an error");
 #endif // #if 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:
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());
+#else
+  llvm_unreachable("GetData() should have returned an error");
 #endif // #if LLDB_ENABLE_TERMIOS
 }
 
 llvm::Error Terminal::SetStopBits(unsigned int stop_bits) {
   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());
+#else
+  llvm_unreachable("GetData() should have returned an error");
 #endif // #if LLDB_ENABLE_TERMIOS
 }
 
 llvm::Error Terminal::SetParity(Terminal::Parity parity) {
   llvm::Expected<Data> data = GetData();
   if (!data)
     return data.takeError();
 

 #else
       return llvm::createStringError(
           llvm::inconvertibleErrorCode(),
           "space/mark parity is not supported by the platform");
 #endif
     }
   }
   return SetData(data.get());
+#else
+  llvm_unreachable("GetData() should have returned an error");
 #endif // #if LLDB_ENABLE_TERMIOS
 }
 
 llvm::Error Terminal::SetParityCheck(Terminal::ParityCheck parity_check) {
   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());
+#else
+  llvm_unreachable("GetData() should have returned an error");
 #endif // #if LLDB_ENABLE_TERMIOS
 }
 
 llvm::Error Terminal::SetHardwareFlowControl(bool enabled) {
   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)
+#else
+  llvm_unreachable("GetData() should have returned an error");
 #endif // #if LLDB_ENABLE_TERMIOS
 }
 
 TerminalState::TerminalState(Terminal term, bool save_process_group)
     : m_tty(term) {
   Save(term, save_process_group);
 }