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

[Péter Radics via llvm-commits]

mitchnull created this revision.
mitchnull added a reviewer: joerg.
mitchnull added a subscriber: llvm-commits.

Proposed fix for the reported terminal issue in PR25007.

- Save the current term settings before doing the check for color support.
- Restore the term settings after the tests (or if the test fails).

This fixes the reported issue for me (with vim-clang-complete), and keeps the original terminal color detection working (-f[no-]color-diagnostics).

I chose to duplicate the cleanup logic (set_curterm(OrigTerm)) twice (once on the error path and once on the normal path) instead of restructuring the function to keep the patch simpler.

Another option would be to use the already set up term in case set_curterm returns non-NULL term, but I felt this approach is safer (and closer to the original logic).


http://reviews.llvm.org/D15077

Files:
  lib/Support/Unix/Process.inc

Index: lib/Support/Unix/Process.inc
===================================================================
--- lib/Support/Unix/Process.inc
+++ lib/Support/Unix/Process.inc
@@ -353,11 +353,16 @@
   // First, acquire a global lock because these C routines are thread hostile.
   MutexGuard G(*TermColorMutex);
 
+  // Save the current term (if set) to be restored after the test.
+  // See PR25007.
+  struct term *OrigTerm = set_curterm((struct term *)nullptr);
   int errret = 0;
-  if (setupterm((char *)nullptr, fd, &errret) != 0)
+  if (setupterm((char *)nullptr, fd, &errret) != 0) {
     // Regardless of why, if we can't get terminfo, we shouldn't try to print
     // colors.
+    (void)set_curterm(OrigTerm);
     return false;
+  }
 
   // Test whether the terminal as set up supports color output. How to do this
   // isn't entirely obvious. We can use the curses routine 'has_colors' but it
@@ -375,8 +380,9 @@
   bool HasColors = tigetnum(const_cast<char *>("colors")) > 0;
 
   // Now extract the structure allocated by setupterm and free its memory
-  // through a really silly dance.
-  struct term *termp = set_curterm((struct term *)nullptr);
+  // through a really silly dance. Also restore the previously saved term
+  // here.
+  struct term *termp = set_curterm(OrigTerm);
   (void)del_curterm(termp); // Drop any errors here.
 
   // Return true if we found a color capabilities for the current terminal.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D15077.41400.patch
Type: text/x-patch
Size: 1426 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151130/8badf923/attachment.bin>