feat(dgw) : network scan ip v4 broadcasting #636
Conversation
irvingoujAtDevolution
force-pushed
the
Network-Scan-Boardcast
branch
from
0bc0b8b to
2dd2da5
Compare
There was a problem hiding this comment.
suggestion: This example does use any API of any of our crate so we can remove it.
There was a problem hiding this comment.
Sorry, this was meant to keep as a draft, not really ready for review yet, I was waiting for the other PR to be merged so I can rebase and clean up the code on this branch. Still, thanks for your review, I will fix them.
| Some(e) => match e.kind() { | ||
| std::io::ErrorKind::TimedOut => { | ||
| tracing::info!("Timed out"); | ||
| exit(0); |
There was a problem hiding this comment.
issue: exit(0) is not running any destructor and is generally not necessary.
Note that because this function never returns, and that it terminates the process, no destructors on the current stack or any other thread’s stack will be run. If a clean shutdown is needed it is recommended to only call this function at a known point where there are no more destructors left to run; or, preferably, simply return a type implementing Termination (such as ExitCode or Result) from the main function and avoid this function altogether
suggestion: Use return Ok(()) if you want to exit with a success status code. Alternatively, you used break at the other places, which have the same behavior. Yet another alternative is to just return the error instead of handling it this way. I don’t really see any special logic happening other than breaking out of the loop and return Ok(()) anyway. Maybe you didn’t mean to break the loop at the other places? (In order to continue processing of the remaining results.)
As an aside, a timeout can probably be considered as an error too. (Err instead of Ok.)
crates/network-scanner/Cargo.toml
Outdated
| tokio = { version = "1.35.1", features = ["io-util"] } | ||
| tokio-stream = "0.1.14" |
There was a problem hiding this comment.
question: I don’t see any utility provided by this crate used in the new code. Am I missing something?
suggestion: Use futures-core instead.
crates/network-scanner/Cargo.toml
Outdated
| @@ -8,9 +8,12 @@ publish = false | |||
|
|
|||
| [dependencies] | |||
| anyhow = "1.0.79" | |||
| get_if_addrs = "0.5.3" | |||
There was a problem hiding this comment.
issue: This crate is unmaintained since 2019 (the repository has been archived).
question: Did you consider this? https://lib.rs/crates/if-addrs
This is maintained fork.
irvingoujAtDevolution
force-pushed
the
Network-Scan-Boardcast
branch
from
b4eb993 to
fa38a85
Compare
|
Not Ready for Review! |
irvingoujAtDevolution
force-pushed
the
Network-Scan-Boardcast
branch
from
f04bb5e to
268f6a4
Compare
irvingoujAtDevolution
changed the title
irvingoujAtDevolution
force-pushed
the
Network-Scan-Boardcast
branch
from
f324a80 to
04d1389
Compare
irvingoujAtDevolution
changed the title
|
Blocked By #665 |
|
Converted to draft until #665 is merged |
|
@irvingoujAtDevolution Can you rebase on top of master? I recommend you to squash your commits before doing so, to avoid conflict hell. |
Supporting network scan ipv4 boardcasting, this is a draft, waiting for #635 to be merged first