[Lldb-commits] [PATCH] D111314: [lldb] [ConnectionFileDescriptorPosix] Use a single NativeFile

[Michał Górny via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdcb0e687faeb: [lldb] [ConnectionFileDescriptorPosix] Use a 
single NativeFile (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111314/new/

https://reviews.llvm.org/D111314

Files:
  lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp


Index: lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
===
--- lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -85,9 +85,8 @@
 : Connection(), m_pipe(), m_mutex(), m_shutting_down(false),
   m_waiting_for_accept(false), m_child_processes_inherit(false) {
   m_write_sp =
-  std::make_shared(fd, File::eOpenOptionWriteOnly, owns_fd);
-  m_read_sp =
-  std::make_shared(fd, File::eOpenOptionReadOnly, false);
+  std::make_shared(fd, File::eOpenOptionReadWrite, owns_fd);
+  m_read_sp = m_write_sp;
 
   Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION |
   LIBLLDB_LOG_OBJECT));
@@ -219,9 +218,8 @@
 m_write_sp = m_read_sp;
   } else {
 m_read_sp = std::make_shared(
-fd, File::eOpenOptionReadOnly, false);
-m_write_sp = std::make_shared(
-fd, File::eOpenOptionWriteOnly, false);
+fd, File::eOpenOptionReadWrite, false);
+m_write_sp = m_read_sp;
   }
   m_uri = std::string(*addr);
   return eConnectionStatusSuccess;
@@ -271,9 +269,8 @@
 }
   }
   m_read_sp =
-  std::make_shared(fd, File::eOpenOptionReadOnly, true);
-  m_write_sp =
-  std::make_shared(fd, File::eOpenOptionWriteOnly, false);
+  std::make_shared(fd, File::eOpenOptionReadWrite, true);
+  m_write_sp = m_read_sp;
   return eConnectionStatusSuccess;
 }
 #endif


Index: lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
===
--- lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -85,9 +85,8 @@
 : Connection(), m_pipe(), m_mutex(), m_shutting_down(false),
   m_waiting_for_accept(false), m_child_processes_inherit(false) {
   m_write_sp =
-  std::make_shared(fd, File::eOpenOptionWriteOnly, owns_fd);
-  m_read_sp =
-  std::make_shared(fd, File::eOpenOptionReadOnly, false);
+  std::make_shared(fd, File::eOpenOptionReadWrite, owns_fd);
+  m_read_sp = m_write_sp;
 
   Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION |
   LIBLLDB_LOG_OBJECT));
@@ -219,9 +218,8 @@
 m_write_sp = m_read_sp;
   } else {
 m_read_sp = std::make_shared(
-fd, File::eOpenOptionReadOnly, false);
-m_write_sp = std::make_shared(
-fd, File::eOpenOptionWriteOnly, false);
+fd, File::eOpenOptionReadWrite, false);
+m_write_sp = m_read_sp;
   }
   m_uri = std::string(*addr);
   return eConnectionStatusSuccess;
@@ -271,9 +269,8 @@
 }
   }
   m_read_sp =
-  std::make_shared(fd, File::eOpenOptionReadOnly, true);
-  m_write_sp =
-  std::make_shared(fd, File::eOpenOptionWriteOnly, false);
+  std::make_shared(fd, File::eOpenOptionReadWrite, true);
+  m_write_sp = m_read_sp;
   return eConnectionStatusSuccess;
 }
 #endif
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D111314: [lldb] [ConnectionFileDescriptorPosix] Use a single NativeFile

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D111314#3050392 , @mgorny wrote:

> In D111314#3050383 , @labath wrote:
>
>> At one point these didn't use to be shared pointers so we couldn't copy them 
>> like this, though I am still unclear as to why we need two of them in the 
>> first place.
>
> Actually, I've asked you on IRC yesterday if you think I should replace them 
> with one field, or if we should keep the separate semantics for some possible 
> use in the future.

Yeah, and I don't really know the answer to that.

But if you make a patch to merge the two,  I'll probably approve it. :)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111314/new/

https://reviews.llvm.org/D111314

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D111314: [lldb] [ConnectionFileDescriptorPosix] Use a single NativeFile

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

In D111314#3050383 , @labath wrote:

> At one point these didn't use to be shared pointers so we couldn't copy them 
> like this, though I am still unclear as to why we need two of them in the 
> first place.

Actually, I've asked you on IRC yesterday if you think I should replace them 
with one field, or if we should keep the separate semantics for some possible 
use in the future.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111314/new/

https://reviews.llvm.org/D111314

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D111314: [lldb] [ConnectionFileDescriptorPosix] Use a single NativeFile

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

At one point these didn't use to be shared pointers so we couldn't copy them 
like this, though I am still unclear as to why we need two of them in the first 
place.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111314/new/

https://reviews.llvm.org/D111314

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D111314: [lldb] [ConnectionFileDescriptorPosix] Use a single NativeFile

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: labath, teemperor, krytarowski, emaste.
mgorny requested review of this revision.

Replace separate read and write NativeFile instances with a single
instance shared for reading and writing.  There is no clear indication
why two instances were used in the first place, and replacing them
with just one does not seem to cause any regressions in tests or manual
'process connect file://...'.


https://reviews.llvm.org/D111314

Files:
  lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp


Index: lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
===
--- lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -85,9 +85,8 @@
 : Connection(), m_pipe(), m_mutex(), m_shutting_down(false),
   m_waiting_for_accept(false), m_child_processes_inherit(false) {
   m_write_sp =
-  std::make_shared(fd, File::eOpenOptionWriteOnly, owns_fd);
-  m_read_sp =
-  std::make_shared(fd, File::eOpenOptionReadOnly, false);
+  std::make_shared(fd, File::eOpenOptionReadWrite, owns_fd);
+  m_read_sp = m_write_sp;
 
   Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION |
   LIBLLDB_LOG_OBJECT));
@@ -219,9 +218,8 @@
 m_write_sp = m_read_sp;
   } else {
 m_read_sp = std::make_shared(
-fd, File::eOpenOptionReadOnly, false);
-m_write_sp = std::make_shared(
-fd, File::eOpenOptionWriteOnly, false);
+fd, File::eOpenOptionReadWrite, false);
+m_write_sp = m_read_sp;
   }
   m_uri = std::string(*addr);
   return eConnectionStatusSuccess;
@@ -259,9 +257,8 @@
 }
   }
   m_read_sp =
-  std::make_shared(fd, File::eOpenOptionReadOnly, true);
-  m_write_sp =
-  std::make_shared(fd, File::eOpenOptionWriteOnly, false);
+  std::make_shared(fd, File::eOpenOptionReadWrite, true);
+  m_write_sp = m_read_sp;
   return eConnectionStatusSuccess;
 }
 #endif


Index: lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
===
--- lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -85,9 +85,8 @@
 : Connection(), m_pipe(), m_mutex(), m_shutting_down(false),
   m_waiting_for_accept(false), m_child_processes_inherit(false) {
   m_write_sp =
-  std::make_shared(fd, File::eOpenOptionWriteOnly, owns_fd);
-  m_read_sp =
-  std::make_shared(fd, File::eOpenOptionReadOnly, false);
+  std::make_shared(fd, File::eOpenOptionReadWrite, owns_fd);
+  m_read_sp = m_write_sp;
 
   Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION |
   LIBLLDB_LOG_OBJECT));
@@ -219,9 +218,8 @@
 m_write_sp = m_read_sp;
   } else {
 m_read_sp = std::make_shared(
-fd, File::eOpenOptionReadOnly, false);
-m_write_sp = std::make_shared(
-fd, File::eOpenOptionWriteOnly, false);
+fd, File::eOpenOptionReadWrite, false);
+m_write_sp = m_read_sp;
   }
   m_uri = std::string(*addr);
   return eConnectionStatusSuccess;
@@ -259,9 +257,8 @@
 }
   }
   m_read_sp =
-  std::make_shared(fd, File::eOpenOptionReadOnly, true);
-  m_write_sp =
-  std::make_shared(fd, File::eOpenOptionWriteOnly, false);
+  std::make_shared(fd, File::eOpenOptionReadWrite, true);
+  m_write_sp = m_read_sp;
   return eConnectionStatusSuccess;
 }
 #endif
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits