Gping with no args

[sylvia-ou]

I'm a student going into my junior year in computer science. I'm completely new to rust but I have some experience in java and a little in assembler. I would like to learn more about rust and maybe contribute to gping.

I want to make gping more user friendly because it's a very useful tool for troubleshooting Wi-Fi and Internet problems. However, the default behavior of gping going to a blank screen when no arguments are given is not friendly to casual users. It would be great if gping could plot default gateway and first ISP hop when no arguments are given.

So far, I've written some rust code that uses the tracert command to determine the first two hops and save the two corresponding IP addresses into two string variables (code down below). I want to insert these two variables in as the default command line host arguments, but I'm not having any luck. Any help or hints would be appreciated.

Please excuse the sloppy code. I need to add conditionals to this to avoid using hops with private IPs and potentially add support for MacOS users since I work on Windows.

fn main() { //TODO: //Private IPs to avoid: 192.168.0.0/16, 172.16.0.0/12, 10.0.0.0/8 //Support for MacOS users

use std::process::Command;
let output = if cfg!(target_os = "windows") {
    Command::new("cmd")
            .args(&["/C", "tracert -d -h 2 google.com"])
            .output()
            .expect("failed to execute process")
} else {
    Command::new("sh")
            .arg("-c")
            .arg("echo hello")
            .output()
            .expect("failed to execute process")
};

let trace = String::from_utf8_lossy(&output.stdout);

let mut line_count = 0;
let mut first = "";
let mut second = "";

for line in trace.lines()
{
    line_count = line_count + 1;
    if line_count == 5
    {
        first = line;
    }
    if line_count == 6
    {
        second = line;
    }
}

let mut token_count = 0;

for token in first.split_whitespace()
{
    token_count = token_count + 1;
    if token_count == 8
    {
        first = token;
    }
}
token_count = 0;
for token in second.split_whitespace()
{
    token_count = token_count + 1;
    if token_count == 8
    {
        second = token;
    }
}

//println!("1: {}", first);
//println!("2: {}", second);

let run = if cfg!(target_os = "windows") {
    Command::new("cmd")
            .args(&["/C", "gping -s ", first, second])
            .output()
            .expect("failed to execute process")
} else {
    Command::new("sh")
            .arg("-c")
            .arg("echo hello")
            .output()
            .expect("failed to execute process")
};

}

[orf]

Hey! Sorry for leaving this unanswered for so long, I've been rather busy.

gping not doing anything when you invoke it with no arguments was unintentional, I actually just fixed this here https://github.com/orf/gping/commit/c9c52b6593ebc18fba05972f13aa61b1cb2fd043#diff-9e16291dbd70f50044762d631f332064e7138836833efccae55eab0a58f28e39R47-R50. Now you need to give it some arguments.

I'm not against adding a sensible default thing to ping if you don't give it any arguments, and the first hop after your router seems like an interesting candidate. However there are always trade-offs when adding a feature like this and you need to evaluate the cost vs the benefit, which is something you'll get used to (or sick of) hearing once you graduate and start working on larger systems 😄.

Parsing the output of ping is non-trivial and annoying. We could add support for parsing the output of traceroute as well, but I expect this this would be even more complex than ping and have a lot more corner cases. For example, what if you're running this from within a Docker container? In this case there is an extra first hop (your local machine). There also might be better, OS-specific ways of getting this information that don't rely on traceroute. So I'm not going to add this, as the maintenance burden would be significant and it would add a whole new dimension to the support matrix (both ping + traceroute need to be compatible).

Perhaps adding a good default target to ping would be more user-friendly? Google has a lot of geographically distributed anycast IPs we could use. I think adding 8.8.8.8 as a default would suffice - it should always be a few hops way from the user. If you did want to explore this feature further, I'd suggest trying to make it a separate utility/command, so you could maybe run something like this: gping $(sylvia-ous-cool-tool --hops=2), which would work because your tool would output the 2nd hop to stdout.

As for the rust code, I'm definitely no expert but I can offer some constructive criticism. Firstly, Rust iterators are really, really awesome. In your first loop you're trying to get the 6th and 7th lines of the output, which can be done like so:

let trace = String::from_utf8_lossy(&output.stdout);
let mut iterator = trace.lines().iter()
let first = iterator.nth(5);
let second = iterator.next();

I'm not sure what the next statements are doing, but perhaps a regular expression can help here? Whenever I find myself doing something that looks like that it's a good sign that it's time to make a regex rather than use fragile things like counting tokens. However if you did want to keep doing it as-is, you could either use nth() like above or you can use enumerate() which gives you the index and removes the need for the token_count variable:

for (idx, token) in first.split_whitespace() {
    if idx == 8 {
        // do stuff
    }
}

I hope this helps. I'm going to close this issue now, but I'm always happy to help people new to Rust (or comp-sci in general) so feel free to reply if anything isn't clear or you have any questions.

[sylvia-ou]

Hi!

Thank you so much for the help! Any of the code that I had was based very heavily off of whatever I found on google so this helps a lot!

Sylvia

On Mon, Jul 19, 2021 at 3:25 PM Tom Forbes @.***> wrote:

Hey! Sorry for leaving this unanswered for so long, I've been rather busy.

gping not doing anything when you invoke it with no arguments was unintentional, I actually just fixed this here c9c52b6

diff-9e16291dbd70f50044762d631f332064e7138836833efccae55eab0a58f28e39R47-R50

https://github.com/orf/gping/commit/c9c52b6593ebc18fba05972f13aa61b1cb2fd043#diff-9e16291dbd70f50044762d631f332064e7138836833efccae55eab0a58f28e39R47-R50. Now you need to give it some arguments.

I'm not against adding a sensible default thing to ping if you don't give it any arguments, and the first hop after your router seems like an interesting candidate. However there are always trade-offs when adding a feature like this and you need to evaluate the cost vs the benefit, which is something you'll get used to (or sick of) hearing once you graduate and start working on larger systems 😄.

Parsing the output of ping is non-trivial and annoying. We could add support for parsing the output of traceroute as well, but I expect this this would be even more complex than ping and have a lot more corner cases. For example, what if you're running this from within a Docker container? In this case there is an extra first hop (your local machine). There also might be better, OS-specific ways of getting this information that don't rely on traceroute. So I'm not going to add this, as the maintenance burden would be significant and it would add a whole new dimension to the support matrix (both ping + traceroute need to be compatible).

Perhaps adding a good default target to ping would be more user-friendly? Google has a lot of geographically distributed anycast IPs https://www.zenlayer.com/blog/anycast-2/ we could use. I think adding 8.8.8.8 as a default would suffice - it should always be a few hops way from the user. If you did want to explore this feature further, I'd suggest trying to make it a separate utility/command, so you could maybe run something like this: gping $(sylvia-ous-cool-tool --hops=2), which would work because your tool would output the 2nd hop to stdout.

As for the rust code, I'm definitely no expert but I can offer some constructive criticism. Firstly, Rust iterators are really, really awesome. In your first loop you're trying to get the 6th and 7th lines of the output, which can be done like so:

let trace = String::from_utf8_lossy(&output.stdout); let mut iterator = trace.lines().iter() let first = iterator.nth(5); let second = iterator.next();

I'm not sure what the next statements are doing, but perhaps a regular expression can help here? Whenever I find myself doing something that looks like that it's a good sign that it's time to make a regex rather than use fragile things like counting tokens. However if you did want to keep doing it as-is, you could either use nth() like above or you can use enumerate() which gives you the index and removes the need for the token_count variable:

for (idx, token) in first.split_whitespace() {

if idx == 8 {

    // do stuff

}

}

I hope this helps. I'm going to close this issue now, but I'm always happy to help people new to Rust (or comp-sci in general) so feel free to reply if anything isn't clear or you have any questions.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/orf/gping/issues/139#issuecomment-882901776, or unsubscribe https://github.com/notifications/unsubscribe-auth/ASWDOVYNLYI34OKTYBYK5U3TYSQ5DANCNFSM4735K7HQ .