[Lldb-commits] [PATCH] D60152: Fix and simplify PrepareCommandsForSourcing

ted added a comment. llvm/tools/lldb/tools/driver/Driver.cpp:495:12: error: comparison of integers of different signs: 'ssize_t' (aka 'long') and 'size_t' (aka 'unsigned long') [-Werror,-Wsign-compare] if (nrwr != commands_size) { ^ ~ 1 error generated. Repository:

[Lldb-commits] [PATCH] D60152: Fix and simplify PrepareCommandsForSourcing

zturner added inline comments. Comment at: lldb/tools/driver/Driver.cpp:492-496 #ifdef _WIN32 - _close(fds[WRITE]); - fds[WRITE] = -1; + _close(fd); #else - close(fds[WRITE]); - fds[WRITE] = -1; + close(fd); #endif labath wrote: >

[Lldb-commits] [PATCH] D60152: Fix and simplify PrepareCommandsForSourcing

This revision was automatically updated to reflect the committed changes. Closed by commit rL357626: Fix and simplify PrepareCommandsForSourcing (authored by amccarth, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews

[Lldb-commits] [PATCH] D60152: Fix and simplify PrepareCommandsForSourcing

labath accepted this revision. labath added inline comments. Comment at: lldb/tools/driver/Driver.cpp:492-496 #ifdef _WIN32 - _close(fds[WRITE]); - fds[WRITE] = -1; + _close(fd); #else - close(fds[WRITE]); - fds[WRITE] = -1; + close(fd); #endif --

[Lldb-commits] [PATCH] D60152: Fix and simplify PrepareCommandsForSourcing

amccarth updated this revision to Diff 193515. amccarth edited the summary of this revision. amccarth added a comment. Further simplification per labath's feedback. Specifically: 1. Use a utility function from llvm to avoid making own wrapper for closing a file descriptor. 2. Don't bother reset

[Lldb-commits] [PATCH] D60152: Fix and simplify PrepareCommandsForSourcing

amccarth marked an inline comment as done. amccarth added a comment. Thanks for the review. In D60152#1452704 , @labath wrote: > FTR, I believe using pipes here is wrong altogether because it can lead to > deadlock. The size of the pipe buffer is impleme

[Lldb-commits] [PATCH] D60152: Fix and simplify PrepareCommandsForSourcing

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Nice cleanup, thanks. I have one suggestion inline you can implement if you think it's a good idea. FTR, I believe using pipes here is wrong altogether because it can lead to deadlock. The si

[Lldb-commits] [PATCH] D60152: Fix and simplify PrepareCommandsForSourcing

amccarth created this revision. amccarth added reviewers: zturner, labath, rnk. Spotted some problems in the Driver's PrepareCommandsForSourcing while helping a colleague track another problem. 1. One error case was not handled because there was no else clause. Fixed by switching to llvm's early