1318428 - Use nsstring bindings in rust-url-capi

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Manish Goregaokar [:manishearth]]

We should use Michael's nsstring bindings in rust-url-capi instead of the c_fn_get_buffer stuff.

Comment 1

[Nika Layzell [:nika] (ni? for response)]

Attachment: Use the nsstring bindings in rust-url-capi

This patch also drops the pretense that rust-url-capi will be used from
outside of c++, or that it will be used outside of mozilla-central,
removing the ifdef __cplusplus code, and including the C++ header
"nsString.h".

MozReview-Commit-ID: BULhHf3DObe

Comment 2

[Manish Goregaokar [:manishearth]]

Comment on attachment 8812790 [details] [diff] [review]
Use the nsstring bindings in rust-url-capi

Review of attachment 8812790 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM. Needs peer review to land.

Have you tested it with network logging to see if it's still largely working? It shouldn't complain about mismatches on regular sites but will complain with malformed IDNA like https://github.com/servo/rust-url/issues/207

Comment 3

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8812790 [details] [diff] [review]
Use the nsstring bindings in rust-url-capi

Review of attachment 8812790 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. Please do a try run before pushing.

::: netwerk/base/rust-url-capi/src/lib.rs
@@ +70,4 @@
>    if urlptr.is_null() {
>      return NSError::InvalidArg.error_code();
>    }
> +  let url = &*(urlptr as *const Url);

Is there an advantage to using this syntax instead of let url: &Url = mem::transmute(urlptr); ?

Comment 4

[Manish Goregaokar [:manishearth]]

> Is there an advantage to using this syntax instead of let url: &Url = mem::transmute(urlptr); ?

Generally good style to avoid transmuting when you can do it a different way. Transmutes are a swiss army knife, and just as pointy -- if the type of the argument changed here you may end up transmuting the wrong thing, whereas you'd be protected from that to some degree with this.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bd1c7797c866
Use the nsstring bindings in rust-url-capi, r=valentin

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/bd1c7797c866