This is an archive of the discontinued LLVM Phabricator instance.

Set stdout/stdin to binary mode on Windows
ClosedPublic

Authored by lanza on Sep 28 2018, 2:14 PM.

Details

Summary

A file opened in text mode on Windows will have \n automatically changed to 13,10 while Darwin and Linux leave it as 10.

Set the file to binary mode to avoid this automatic conversion so that Darwin, Linux and Windows have equivalent treatment of \r.

Diff Detail

Repository
rLLDB LLDB

Event Timeline

lanza created this revision.Sep 28 2018, 2:14 PM
clayborg added a comment.EditedSep 28 2018, 2:41 PM

We might need to open the file in binary mode to avoid \n from being converted to two characters in the JSON payload itself. So maybe this fix should be that we open the file in binary mode?

lanza updated this revision to Diff 167568.Sep 28 2018, 4:52 PM

swap to just changing stdout & stdin

lanza abandoned this revision.Sep 28 2018, 4:54 PM

The VSCode.txt part looks good without the ifdef to set binary mode. Are you gonna make another patch with just that fix?

tools/lldb-vscode/VSCode.cpp
47–50

No need for ifdef here. Just do it for all platforms.

lanza updated this revision to Diff 167809.Oct 1 2018, 1:24 PM

rebase properly

lanza added a comment.Oct 1 2018, 1:29 PM

No need for ifdef here. Just do it for all platforms.

So the BSD/Darwin setmode is not the same as the Windows _setmode (e.g. only one argument vs two on Windows). Linux doesn't even have a setmode.

The VSCode.txt part looks good without the ifdef to set binary mode. Are you gonna make another patch with just that fix?

I'm not sure what you mean by this. I messed up the rebase on the previous revision, maybe you're talking about that?

lanza retitled this revision from Switch SendJSON to use just \n on Windows to Set stdout/stdin to binary mode on Windows.Oct 1 2018, 1:31 PM
lanza edited the summary of this revision. (Show Details)
clayborg accepted this revision.Oct 2 2018, 9:10 AM
This revision is now accepted and ready to land.Oct 2 2018, 9:10 AM

Be sure to add a comment specifying why we are doing this only for windows. We might be able to remove the #if and do it for all platforms.

tools/lldb-vscode/VSCode.cpp
47

Add a comment specifying why we are doing this.

lanza updated this revision to Diff 168705.Oct 8 2018, 1:30 PM

add a comment

lanza marked 2 inline comments as done.Oct 8 2018, 1:57 PM
lanza added a reviewer: xiaobai.
clayborg accepted this revision.Oct 9 2018, 7:09 AM
This revision was automatically updated to reflect the committed changes.