1

Here is my code:

    def ping_host(self, hostname, ping_cmd='/usr/bin/ping', count=1, timeout=400):
        cmd = [ping_cmd, '-c%s' % count, '-W%s' % timeout, hostname]
        (output, error) = subprocess.Popen(cmd,
                                           stdout=subprocess.PIPE,
                                           stderr=subprocess.PIPE,
                                           shell=True).communicate()
        print ' '.join( cmd )
        print output, error

When I run it I get this output:

/usr/bin/ping -c1 -W400 tools-dev1.example.com
 Usage: ping [-aAbBdDfhLnOqrRUvV64] [-c count] [-i interval] [-I interface]
            [-m mark] [-M pmtudisc_option] [-l preload] [-p pattern] [-Q tos]
            [-s packetsize] [-S sndbuf] [-t ttl] [-T timestamp_option]
            [-w deadline] [-W timeout] [hop1 ...] destination
Usage: ping -6 [-aAbBdDfhLnOqrRUvV] [-c count] [-i interval] [-I interface]
             [-l preload] [-m mark] [-M pmtudisc_option]
             [-N nodeinfo_option] [-p pattern] [-Q tclass] [-s packetsize]
             [-S sndbuf] [-t ttl] [-T timestamp_option] [-w deadline]
             [-W timeout] destination

I assume I am doing something wrong with the first argument, cmd, I am passing to Popen(), but I don't know what. If I cut-n-paste /usr/bin/ping -c1 -W400 tools-dev1.example.com to a command line it works fine.

Red Cricket
  • 9,762
  • 21
  • 81
  • 166
  • 3
    Try without `shell=true`. If you want `shell=true`, you should be passing a *string* command rather than a list of arguments. – Aran-Fey Feb 26 '19 at 21:30
  • 2
    @Torxed, the `shlex.split()` form reintroduces bugs you'd otherwise be avoiding by not having a shell -- one needs to worry again about whether substituted values contain whitespace, f/e. – Charles Duffy Feb 26 '19 at 21:37
  • @CharlesDuffy Arguing over bug prone code is nonsense to me. Everything has it's usecases, and in this case `shlex.split('ping -c1 -w400 tools-dev1.example.com')` won't produce any. It would make life a littler easier to read the code and arguably lowering code complexity minimizes the risk of bugs just as much as preaching about shlex being dangerous in other situations. In this use case, I didn't see the harm in using it - there for - I suggested it to make the command line more readable. Especially if using `shell=True`* wasn't an option and a list needed to be generated *from* a string. – Torxed Feb 26 '19 at 21:49
  • @Torxed, ...I assume you've been here long enough to hear my war story about a major data loss event caused by someone assuming a directory "couldn't ever" contain names that with non-hex characters, and thus not bothering to quote those names, but this is the same category of bug; if someone took the practice you showed above and used it with filenames rather than hostnames, we're right in that class of scenario. – Charles Duffy Feb 26 '19 at 21:56
  • @CharlesDuffy This is ridiculous, you're telling me that I have to explain all the facts and inner workings of X - or not at all? While you yourself [barely](https://stackoverflow.com/a/54718218/929999) touch on topics regarding `readline()` being dangerous on some python versions or that not all empty lines starts with `\n` on all systems/encodings giving false results? You're right, I'll stop giving out free advice on the internet if I don't supply a 100+ doctorate thesis on the times it could go wrong. Use case and context matters to me. And it was a comment, not an answer for this reason. – Torxed Feb 26 '19 at 22:08
  • I appreciate the catch (though it might have been a better comment on the answer to which it applies, rather than here). Beyond that, this comment thread appears to have been mooted. Suffice to say that I agree that context matters, but I continue to disagree -- strongly -- that describing a practice prone to causing errors if widely applied next to a generally sound one in the context of entry-level documentation is wise. – Charles Duffy Feb 26 '19 at 22:27

0 Answers0