[Lldb-commits] [PATCH] D99426: [Windows] Add new OF_TextWithCRLF flag and use this flag instead of OF_Text
Abhina Sree via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Tue Mar 30 10:13:44 PDT 2021
abhina.sreeskantharajan added inline comments.
Comment at: llvm/lib/Support/Windows/Path.inc:1086
- if (Flags & OF_Text)
+ if (Flags & OF_CRLF)
CrtOpenFlags |= _O_TEXT;
> I assume it would be weird to set `OF_CRLF` without also setting `OF_Text`, so this change seems right for correctness. But maybe this Windows-only code would better express the intent if it were written:
> if (Flags & (OF_CRLF | OF_Text))
> Another idea would be to assert if `OF_CRLF` is set but `OF_Text` is not.
> Or maybe it's fine as it is.
I chose to create OF_CRLF flag because I only want Windows to turn on text mode for OF_TextWithCRLF and not OF_Text.
This solution would return true even if Flags was just OF_Text.
if (Flags & (OF_CRLF | OF_Text))
I think adding an assertion here would be a good idea.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the lldb-commits