Upgrade bindgen

[jethrogb]

This crate is currently using an old version of bindgen which requires LLVM 3.8 and no newer. The API and behavior has changed significantly since then, so it'll be a bit of work to upgrade.

[ekse]

I would like to give this a try. Since Builder::remove_prefix was removed, are you ok with using names with the "mbedtls_" prefix, except when we reexport values?

[jethrogb]

Oh I forgot about this issue. See rust-lang/rust-bindgen#428 . I'd really like to automate this since I'm not a big fan of having to use the prefixes everywhere.

[ekse]

I worked on this issue in the last few days, here is a status update on my progress so far. I would like to discuss potential approaches to deal with issues I encountered. You can find my work branch at https://github.com/ekse/rust-mbedtls/tree/bindgen-upgrade.

First, a quick note on why I want to upgrade. The current master branch does not compile on my computers, it fails with the error Bindgen ERROR: unsupported type CXTypeKind(119). Upgrading bindgen fixes this issue. I get this build error on libclang 3.9, 4.0 and 6.0.

"mbedtls_" prefix

To deal with the "mbedtls_" prefix, I tried the approach suggested by the bindgen maintainer to rename the imports with use x as y statements. To reduce the amount of boilerplate, I experimented with creating a procedural macro "mbedtls_use" https://github.com/ekse/rust-mbedtls/tree/bindgen-upgrade/mbedtls-use. An example use of this macro looks like this: (https://github.com/ekse/rust-mbedtls/blob/bindgen-upgrade/mbedtls/src/cipher/raw/serde.rs#L24)

#[mbedtls_use]
use {
    mbedtls_aes_context, mbedtls_cipher_context_t, mbedtls_cipher_id_t, mbedtls_cipher_mode_t,
    mbedtls_des3_context, mbedtls_des_context, MBEDTLS_CIPHER_ID_3DES, MBEDTLS_CIPHER_ID_AES,
    MBEDTLS_CIPHER_ID_DES, MBEDTLS_MODE_CBC, MBEDTLS_MODE_CFB, MBEDTLS_MODE_CTR,
};

There are a couple downsides to this approach. Procedural macros were just stabilized in Rust 1.30, that would make it a hard requirement. Also, the use statements are not in scope when using macros like define!, so I used the full type names (see https://github.com/ekse/rust-mbedtls/blob/bindgen-upgrade/mbedtls/src/pk/ec.rs#L19).

Alternatively, we could remove the proc_macro and write the complete use x as y statements, at the cost of longer use statements.

unsigned constants

The second issue I encountered is with constants. Bindgen defines constants such as MBEDTLS_SSL_IS_CLIENT as u32. This fails to compile for define! statements that map to c_int as it maps to i32. I assume the older version of bindgen defined those constants as i32.

error[E0308]: match arms have incompatible types                                                                                            
  --> mbedtls/src/wrapper_macros.rs:75:5                                                                                                    
   |                                                                                                                                        
75 |                   match self {                                                                                                         
   |  _________________^                                                                                                                    
76 | |                     $($n::$rust => ::mbedtls_sys::$c,)*                                                                              
   | |                                    ----------------- match arm with an incompatible type                                             
77 | |                 }                                                                                                                    
   | |_________________^ expected i32, found u32                                                                                            
   |                                                                                                                                        
  ::: mbedtls/src/ssl/config.rs:62:1                                                                                                        
   |                                                                                                                                        
62 | / define!(enum UseSessionTickets -> c_int {                                                                                            
63 | |     Enabled => MBEDTLS_SSL_SESSION_TICKETS_ENABLED,                                                                                  
64 | |     Disabled => MBEDTLS_SSL_SESSION_TICKETS_DISABLED,                                                                                
65 | | });                                                                                                                                  
   | |___- in this macro invocation  

Also, some getters and setters fail to compile because enums do not implement From conversions for i32 (because values in u32 could be outside the range of i32).

setter!(set_session_tickets(u: UseSessionTickets) = mbedtls_ssl_conf_session_tickets)

error[E0277]: the trait bound `i32: std::convert::From<ssl::config::UseSessionTickets>` is not satisfied                                    
   --> mbedtls/src/wrapper_macros.rs:192:50                                                                                                 
    |                                                                                                                                       
192 |             unsafe{::mbedtls_sys::$cfn(&mut self.inner,$n.into())}                                                                    
    |                                                           ^^^^ the trait `std::convert::From<ssl::config::UseSessionTickets>` is not implemented for `i32`

I temporarily worked around this by manually defining the constants as i32, and the crate successfully compiles at that point. To could be an approach to deal with this issue, I'm not sure what the best way to do that with bindgen would be.

---

Sorry for the long message, it seems upgrading the bindgen dependency will require a good amount of effort. I suggest we decide on a way to deal with the mbedtls_ prefix issue and move on from there.

Thanks,

Sébastien

[jethrogb]

prefix

If you take a look at the latest comments at rust-lang/rust-bindgen#428 it seems to me that reimplementing remove_prefix shouldn't be too hard. That would have my preference over procedural macros.

constants

I think it should be pretty easy to replace the current macro_int_types configuration with an appropriate ParseCallbacks::int_macro implementation.

[ekse]

Thanks for the reply, I had not seen the reply on the bindgen issue. I implemented an item_name` callback and it is now merged in the master branch of bindgen.

I pushed a new branch with your suggested approaches, the changeset is now much much smaller.
master...ekse:bindgen-upgrade-v2

client_server_testis failing with X509CertVerifyFailed on the client. I will try to figure out why.

[ekse]

Haha I found it: The certificate is expired, the expiration date is '16 Aug 2016'.

[jethrogb]

@ekse any progress? Do you need help figuring anything out?

[ekse]

I opened a pull request (#13) to replace the expired certificate in the tests.

For the bindgen upgrade, I was waiting for a new release of bindgen. I asked the maintainers today if they could make a new release. My branch currently compiles and all the tests are passing, it depends on the git repository of bindgen at the moment, so once the bindgen crate is updated it will be ready for a pull request.
master...ekse:bindgen-upgrade-v2

[jethrogb]

This is also necessary for the docs to show up properly on docs.rs