[PATCH] D15077: [PATCH] Save and restore current term setting in terminalHasColors(). PR25007

[Péter Radics via llvm-commits]

mitchnull added a comment.

In http://reviews.llvm.org/D15077#300234, @joerg wrote:

> I see two issues with the current code and I think they should be considered as a whole for any chance here.
>
> (1) We should only ever run this dance once. Even with clang we currently run it per source input.


I don't think it's such a big issue, most build systems compile a  single file at a time, not multiple source files on the same command line. I think it's better to keep the most frequent use-case simple (and fast).

> (2) Since the dance is inherently not thread-safe, code using it as library, directly or indirectly, should have a way to either provide a fixed option OR to initialize the state at a safe point.


I glanced at the gnu ncurses code, and apparently it does have some thread-safety (at least cur_term is protected in set_curterm).  Not sure about other curses implementations though.  Still, this patch doesn't make anything worse, imo.

> For (2), preserving the current state like in this patch is still a good idea, but it becomes less crucial.





http://reviews.llvm.org/D15077