On Thu, 2014-12-04 at 09:29 -0500, Colin Walters wrote:
On Thu, Dec 4, 2014, at 09:23 AM, Vratislav Podzimek wrote:
> + # using iutil.execWithRedirect here would mean that if this function was run
> + # multiple times from multiple threads in the same time, the execution would
> + # be serialized because of the program_log lock
> + proc = subprocess.Popen(["ntpdate", "-q", server],
close_fds=True)
- Why not subprocess.call()? (You should be able to pass close_fds=True to it)
I
doesn't seem to have that parameter.
- Is it intentional that you're dropping the equivalent of
stdout=open('/dev/null', 'w'), stderr=subprocess.STDOUT ?
Ooops,
good catch, thanks. Fixing locally.
- Also worth mentioning this fixes a bug where if the provided server name had shell
metacharacters it would fail by executing code instead of returning a clean error
In
general yes, but we check server name to be a valid (and even
resolvable) server name before passing it to this function.
--
Vratislav Podzimek
Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic