bugfix: File::GetWaitableHandle() should call fileno()

[lawrence_danna]

Summary

If the file has m_stream, it may not have a m_descriptor.
GetWaitableHandle() should call GetDescriptor(), which will call
fileno(), so it will get waitable descriptor whenever one is available.

[labath]

Thanks for the fix. This sounds like it should be unit testable. Could add a test for that?

[lawrence_danna]

added a unit test

[labath]

Thanks for adding the test. LGTM, modulo some nits inline.

lldb/unittests/Host/CMakeLists.txt

14

Please keep the list sorted.

lldb/unittests/Host/FileTest.cpp

29–32

nit: change EXPECT_XXX to ASSERT_XXX as it does not make sense to continue the test if the assertions are not true.

[lawrence_danna]

nits fixed

[lawrence_danna]

nits fixed

lldb/source/Host/common/File.cpp

#else
     return fileno(m_stream);
 #endif
   }
 
   // Invalid descriptor and invalid stream, return invalid descriptor.
   return kInvalidDescriptor;
 }
 
-IOObject::WaitableHandle File::GetWaitableHandle() { return m_descriptor; }
+IOObject::WaitableHandle File::GetWaitableHandle() { return GetDescriptor(); }
 
 void File::SetDescriptor(int fd, bool transfer_ownership) {
   if (IsValid())
     Close();
   m_descriptor = fd;
   m_should_close_fd = transfer_ownership;
 }
 

lldb/unittests/Host/CMakeLists.txt

 set (FILES
   ConnectionFileDescriptorTest.cpp
   FileActionTest.cpp
   FileSystemTest.cpp
   HostInfoTest.cpp
   HostTest.cpp
   MainLoopTest.cpp
   NativeProcessProtocolTest.cpp
   ProcessLaunchInfoTest.cpp
   SocketAddressTest.cpp
   SocketTest.cpp
   SocketTestUtilities.cpp
   TaskPoolTest.cpp
+  FileTest.cpp
 )
 
 if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
   list(APPEND FILES
     linux/HostTest.cpp
     linux/SupportTest.cpp
   )
 endif()
 
 add_lldb_unittest(HostTests
   ${FILES}
   LINK_LIBS
     lldbHost
     lldbUtilityHelpers
     LLVMTestingSupport
   )

lldb/unittests/Host/FileTest.cpp

+//===-- FileTest.cpp --------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Host/File.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+TEST(File, GetWaitableHandleFileno) {
+  const auto *Info = testing::UnitTest::GetInstance()->current_test_info();
+
+  llvm::SmallString<128> name;
+  int fd;
+  llvm::sys::fs::createTemporaryFile(llvm::Twine(Info->test_case_name()) + "-" +
+                                         Info->name(),
+                                     "test", fd, name);
+  llvm::FileRemover remover(name);
+  EXPECT_GE(fd, 0);
+
+  FILE *stream = fdopen(fd, "r");
+  EXPECT_TRUE(stream);
+
+  File file(stream, true);
+  EXPECT_EQ(file.GetWaitableHandle(), fd);
+}