[PATCH] D16000: Add "/dev/console" as a special file name to lit

[Dan Liew via llvm-commits]

delcypher added a comment.

@ygao

This approach looks flawed to me because you have an implicit assumption that does not always hold. It looks like you're assuming that setting `r[2]` to `None` (which will happen with your change when the file being redirect to is name `/dev/console`) will mean you **always** write to a console.

This assumption is incorrect because when you set stdout/stderr/stdin in the `Popen` constructor to `None` it means that the relevant parameter will be inherited from the parent which may not be connected to a console. Put another way your patch will work if lit is run from a console without redirecting its stdout/stderr but if you run lit and redirect its stdout/stderr to files this patch won't do want you want it to.

Is there a file you open under Windows that will point to a console (a quick google suggests 'CON' might be)? If so I think would be better just to do

  if r[2] is None:
     if r[0] == '/dev/tty' and os.name == 'nt':
       # Provide direct access to the console to simulate /dev/tty under Windows
       r[2] = open('special_console_file_under_windows', 'r' if index == 0 else 'w')

This is more explicit and the intention is much more clear. The intention is not clear at all from your implementation.


http://reviews.llvm.org/D16000