This is an archive of the discontinued LLVM Phabricator instance.

[lldb] [ConnectionFileDescriptorPosix] Use a single NativeFile
ClosedPublic

Authored by mgorny on Oct 7 2021, 7:31 AM.

Details

Summary

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://...'.

Diff Detail

Event Timeline

mgorny requested review of this revision.Oct 7 2021, 7:31 AM
mgorny created this revision.
labath accepted this revision.Oct 8 2021, 1:13 AM

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.

This revision is now accepted and ready to land.Oct 8 2021, 1:13 AM
mgorny added a comment.Oct 8 2021, 1:14 AM

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.

labath added a comment.Oct 8 2021, 1:31 AM

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. :)

Herald added a project: Restricted Project. · View Herald TranscriptOct 8 2021, 2:26 AM