Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ch8/ch8-resolve: does not build with Rust 1.66 (transmute size mismatch in socket2) #99

Closed
ewenmcneill opened this issue Jan 9, 2023 · 3 comments · May be fixed by #107
Closed

ch8/ch8-resolve: does not build with Rust 1.66 (transmute size mismatch in socket2) #99

ewenmcneill opened this issue Jan 9, 2023 · 3 comments · May be fixed by #107

Comments

@ewenmcneill
Copy link

The ch8/ch8-resolve example no longer builds with stable Rust, because the old socket2 dependency that is pulled in tries to do:

    mem::transmute::<SocketAddrV4, sockaddr_in>(v4);

but that does not work now that sockaddr_in has been expanded to support IPv6:

error[E0512]: cannot transmute between types of different sizes, or dependently-sized types
   --> /Users/ewen/.cargo/registry/src/github.com-1ecc6299db9ec823/socket2-0.3.9/src/sockaddr.rs:153:9
    |
153 |         mem::transmute::<SocketAddrV4, sockaddr_in>(v4);
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: source type: `SocketAddrV4` (48 bits)
    = note: target type: `sockaddr_in` (128 bits)

The only reasonable way forward seemed to be to port this example to use a newer trust-dns client version, which had a newer socket2 dependency, which had been updated to take IPv6 support in Rust into account.

So I've forward ported the example to trust-dns-client (the new name for the crate that is the client) by:

  1. Updating the dependency to be trust-dns-client = { version = "0.21.2", default-features = false }

  2. Updating the use entries from trust_dns to trust_dns_client; and

  3. Due to a deprecation warning, changing let resource = answer.rdata(); to be let resource = answer.data().unwrap(); (data() seems to return an Option(rdata) ; I'm guessing the deprecation is to encourage people to check if there was any rdata())

With those changes it seems to build/run:

ewen@basadi:~/misc/src/rust/rust-in-action/code/ch8/ch8-resolve$ cargo run -- www.rustinaction.com
   Compiling resolve v0.1.0 (/Users/ewen/misc/src/rust/rust-in-action/code/ch8/ch8-resolve)
    Finished dev [unoptimized + debuginfo] target(s) in 0.60s
     Running `target/debug/resolve www.rustinaction.com`
35.185.44.232
ewen@basadi:~/misc/src/rust/rust-in-action/code/ch8/ch8-resolve$ 

Diff below. Like #98 this might be another case where it is worth updating the example to the newer rust compatible code, and indicating in the code comments the reason why it differs from the book text.

Ewen

PS: The book text talks about running "resolve" (eg, top of page 265 in the PDF), but obviously the actual exectuable is ch8-resolve, and by default it's buried in a target/debug directory anyway; fortunately the later instructions are for running it via cargo run which does work.

git diff between book shipped example and working on Rust 1.66 example
ewen@basadi:~/misc/src/rust/rust-in-action/code/ch8/ch8-resolve$ git diff Cargo.toml src/main.rs 
diff --git a/ch8/ch8-resolve/Cargo.toml b/ch8/ch8-resolve/Cargo.toml
index 87d76f7..f6634a2 100644
--- a/ch8/ch8-resolve/Cargo.toml
+++ b/ch8/ch8-resolve/Cargo.toml
@@ -7,4 +7,4 @@ edition = "2018"
 [dependencies]
 rand = "0.6"
 clap = "2.33"
-trust-dns = { version = "0.16", default-features = false }
+trust-dns-client = { version = "0.21.2", default-features = false }
diff --git a/ch8/ch8-resolve/src/main.rs b/ch8/ch8-resolve/src/main.rs
index 2e50cfe..2500091 100644
--- a/ch8/ch8-resolve/src/main.rs
+++ b/ch8/ch8-resolve/src/main.rs
@@ -3,10 +3,10 @@ use std::time::Duration;
 
 use clap::{App, Arg};
 use rand;
-use trust_dns::op::{Message, MessageType, OpCode, Query};
-use trust_dns::rr::domain::Name;
-use trust_dns::rr::record_type::RecordType;
-use trust_dns::serialize::binary::*;
+use trust_dns_client::op::{Message, MessageType, OpCode, Query};
+use trust_dns_client::rr::domain::Name;
+use trust_dns_client::rr::record_type::RecordType;
+use trust_dns_client::serialize::binary::*;
 
 fn main() {
   let app = App::new("resolve")
@@ -63,7 +63,12 @@ fn main() {
 
   for answer in dns_message.answers() {
     if answer.record_type() == RecordType::A {
-      let resource = answer.rdata();
+      // .rdata() is deprecated in modern trust_dns_client:
+      //
+      // warning: use of deprecated associated function `trust_dns_client::rr::Record::rdata`: use `Record::data` instead
+      //
+      // let resource = answer.rdata();
+      let resource = answer.data().unwrap();
       let ip = resource
         .to_ip_addr()
         .expect("invalid IP address received");
ewen@basadi:~/misc/src/rust/rust-in-action/code/ch8/ch8-resolve$
@ckoopmann
Copy link

Just encountered the issue as well.
@ewenmcneill Thanks for the fix. I took the liberty and opened a PR with the solution from your git diff, hope that's okay.

https://github.com/rust-in-action/code/pull/107/files

@ewenmcneill
Copy link
Author

Thanks for the fix. I took the liberty and opened a PR with the solution from your git diff, hope that's okay.

That's fine with me. Note that the author has said that PRs will not be merged. Which is why I stopped making PRs (but maybe other readers will still find the PRs easier to find/work with).

I'm glad to hear the solution helped you!

Ewen

@ewenmcneill
Copy link
Author

Closing issue to clean up my GitHub "issues created" list, since after 18 months it seems clear nothing will happen in this Rust book related repo with the open issues.

(The PR containing this fix -- #107 -- is still open at present, since ckoopmann created that one.)

Ewen

@ewenmcneill ewenmcneill closed this as not planned Won't fix, can't repro, duplicate, stale Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants