-
Notifications
You must be signed in to change notification settings - Fork 48
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
Add a way to set context mode when creating context #134
base: master
Are you sure you want to change the base?
Conversation
…t mode for blocking and Runtime_lock. Also test Runtime_lock
This addresses issue #122 |
@anmonteiro This PR coould be extended with a set_mode function ? |
src/ssl.ml
Outdated
let (lor) = (lor) | ||
end | ||
|
||
(** Allow SSL_write(..., n) to return r with 0 < r < n (i.e. report success |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this comment misplaced? What's doing this exactly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeh, a wrong paste: fixed
src/ssl.ml
Outdated
module Modes = struct | ||
type t = int | ||
|
||
let no_mode = 0x000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you add a link to where these values are defined?
src/ssl.ml
Outdated
@@ -77,6 +77,26 @@ type verify_error = | |||
| Error_v_keyusage_no_certsign | |||
| Error_v_application_verification | |||
|
|||
module Modes = struct |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
alternative design:
- use a variant with all the modes
- accept a list of modes
OR
the modes in the list to get mode
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pro of the list design
- often use in OCAml
- allows for pattern matching
Cons - slow
- allocates
- many more lines of codes.
Moreover, if OCaml allowed pattern matching on "constant", i.e. value known at compile time, pattern matching would still be possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can probably make it non-allocating if it's just pattern matches to convert to int, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you move the tests to a separate PR?
I don't know if this is what you asked in the other PR but I'd prefer:
- 1 PR for
modes
handling - 1 PR with tests for
Runtime_lock
done |
Antonio Nuno Monteiro writes:
@anmonteiro commented on this pull request.
> @@ -77,6 +77,26 @@ type verify_error =
| Error_v_keyusage_no_certsign
| Error_v_application_verification
+module Modes = struct
we can probably make it non-allocating if it's just pattern matches to convert to int, no?
You allocate when you build the list of modes.
And moreover, you need to convert the OCaml constant to the proper power
of 2. I really think the proposed pattern for C <-> OCaml constant is better.
I even usually use cpp to generate the ml file, making the pattern type
safe by design.
…--
Christophe Raffalli
tél: +689 87 23 11 48
web: http://raffalli.eu
Ce mail est signé avec pgp (Pièce jointe signature.asc, clef sur https://raffalli.eu/pgp)
This mail is signed with pgp (Attachment signature.asc, key on https://raffalli.eu/pgp)
|
I have investigate async mode more. It seems that openssl can do async computation even when the user does not require async_job manually, if async mode is enabled (the doc is not really clear). |
…t mode for blocking and Runtime_lock. Also test Runtime_lock
create_context has distinct default mode for blocking and Runtime_lock.
Also add tests for Runtime_lock