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

openssl-sys: declare_std_functions macro #1348

Closed
wants to merge 1 commit into from

Conversation

stbuehler
Copy link
Contributor

Hi,

I have been looking for a way to handle the *mut _ -> *const _ changes in openssl / libressl in a sane way; duplicating all the imports seemed a bad thing.

With proc-macros one could probably invent real crazy syntax, but that would add a lot of dependencies for openssl-sys.

The best thing I could come up with was to define signature schemes (with a single type parameter), which can then be applied using a macro syntax I borrowed from foreign_types. (Macros are not allowed in type position (and name/ident?), so I couldn't get any other "fix this type signature" attempts working).

This will probably add a big merge conflict with the openssl-300 branch, but hopefully makes the process easier in the long run.

Also this changes u8 to c_uchar in a few d2i_* and i2d_ functions; I think this shouldn't break anything (libc::c_uchar should be an alias for u8), but is actually the proper signature.

I checked the imported functions on my local system with my systest-api-items branch, which is based on my ctest out-items branch. After sed -e 's#uint8_t#unsigned char#' the items look the same before and after this PR.

@stbuehler stbuehler force-pushed the sys-common-apis branch 2 times, most recently from 2a8110f to f7c0345 Compare September 30, 2020 11:51
Uses `c_uchar` (-> `unsigned char`) instead of `u8` (-> `uint8_t`) in:
  d2i_PUBKEY, i2d_PKCS12, i2d_PKCS7, i2d_PrivateKey, i2d_PUBKEY, i2d_RSA_PUBKEY,
  i2d_RSAPrivateKey, i2d_RSAPublicKey, i2d_X509_CRL, i2d_X509_REQ,
  i2d_X509_REVOKED, i2d_X509, d2i_PKCS12, d2i_RSA_PUBKEY, d2i_RSAPrivateKey,
  d2i_RSAPublicKey
@stbuehler
Copy link
Contributor Author

Ping :)

I was very happy (and a little bit surprised :) ) how quickly you merged my other -sys PRs, which is why gave this one a try.

I'm not too attached to this PR, but it would be nice to know if you want to consider this or not, i.e. do you think "no way", "perhaps some other time" or "good idea, just not sure about some details"?

I just don't think it makes sense to keep this open much longer; it will quickly result in merge conflicts with other -sys PRs.

@sfackler
Copy link
Owner

Thanks for the PR! I do think it'd be nice to clean the const/mut stuff up a bit, but I think I'd prefer something with syntax closer to the standard function type. Total strawman, but I could imagine something like this:

extern "C" {
    cfg_const! { #[cfg(ossl102)] // the cfg for the const version
        pub fn some_function(foo: maybe_const(pointee_type), bar: u32);
    }
}

@stbuehler
Copy link
Contributor Author

I'd like such syntax too, but I think it can only be done with Procedural Macros; if adding dependencies for that sounds fine, I'm happy to give it a try.

I just tried to come up with simple macro examples to show why it can't work, but it compiled.. so I'll try again - perhaps it can be done with macro_rules.

(Splitting arguments is really hard, as you need to match the tokens building a type without using ty. Splitting multiple functions isn't fun either, as they usually end with a type, which again can't be matched using ty. And you can't use ty (afaik) as once it got matched as ty it can't be split into tokens anymore.)

@stbuehler
Copy link
Contributor Author

Replaced by #1359.

@stbuehler stbuehler closed this Oct 25, 2020
@stbuehler stbuehler deleted the sys-common-apis branch October 25, 2020 16:48
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 this pull request may close these issues.

2 participants