[PATCH] D95230: Save and restore previous terminal after setting the terminal for checking if terminal supports colors.

Pavel Labath via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 4 05:37:13 PST 2021


labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

This change seems obviously correct. We'll still need to be careful about using this functionality on the lldb side though. This fix will only help if there is proper synchronization between using this function and libedit initialization. If we still run into issues, we'll need to either add more synchronization, or do something different altogether.

Regarding testing, I suppose it should be possible to write a unit test which checks that this function really preserves the terminal value, but it's not clear to me how useful would that be....



================
Comment at: llvm/lib/Support/Unix/Process.inc:357-358
 
   // Now extract the structure allocated by setupterm and free its memory
   // through a really silly dance.
+  struct term *termp = set_curterm(previous_term);
----------------
Now that its purpose is also to restore the terminal value, this code is no longer that "silly".
I'd update this to something like:
```
// Restore the current terminal to the original value. Be sure to also free the structure allocated by setupterm.
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95230/new/

https://reviews.llvm.org/D95230



More information about the llvm-commits mailing list