Re: Reordering new API's that have a libctx, propq

2020-09-21 Thread Matt Caswell
On 21/09/2020 10:59, Matt Caswell wrote: > > > On 16/09/2020 16:56, Matt Caswell wrote: >>> "Adopt the coding style policy on function arguments as shown in chapter >>> 6.1 of web PR 194 (commit 7b45b46d71f)" > > This vote failed: > > accepted: no (for: 2, against: 5, abstained: 2, not

Re: Reordering new API's that have a libctx, propq

2020-09-21 Thread Matt Caswell
On 16/09/2020 16:56, Matt Caswell wrote: >> "Adopt the coding style policy on function arguments as shown in chapter >> 6.1 of web PR 194 (commit 7b45b46d71f)" This vote failed: accepted: no (for: 2, against: 5, abstained: 2, not voted: 2) >> >> "Adopt the coding style policy on extending

Re: Reordering new API's that have a libctx, propq

2020-09-16 Thread Matt Caswell
On 15/09/2020 12:25, Matt Caswell wrote: > I plan to start two OTC votes on this tomorrow with the following wording: These votes have now commenced. I'll report back with the results when they are known. Matt > > "Adopt the coding style policy on function arguments as shown in chapter >

Re: Reordering new API's that have a libctx, propq

2020-09-15 Thread Matt Caswell
On 14/09/2020 11:30, Matt Caswell wrote: > In order to try and move this discussion forward I have made a concrete > proposal for how we should formulate the various ideas in this thread > into an actual style. Please see here: > > https://github.com/openssl/web/pull/194 I've updated this PR

Re: Reordering new API's that have a libctx, propq

2020-09-14 Thread Richard Levitte
On Mon, 14 Sep 2020 13:46:01 +0200, Tim Hudson wrote: > > [1 ] > [2 ] > On Mon, Sep 14, 2020 at 9:19 PM Matt Caswell wrote: > > I must be misunderstanding your point because I don't follow your logic > at all. > > So this is the correct form according to my proposed policy: >

Re: Reordering new API's that have a libctx, propq

2020-09-14 Thread Matt Caswell
On 14/09/2020 13:14, Tim Hudson wrote: > On Mon, Sep 14, 2020 at 9:52 PM Matt Caswell > wrote: > > > And that is the point - this is not how the existing CTX functions > work > > (ignoring the OPENSSL_CTX stuff). > > Do you have some concrete examples

Re: Reordering new API's that have a libctx, propq

2020-09-14 Thread Tim Hudson
On Mon, Sep 14, 2020 at 9:52 PM Matt Caswell wrote: > > And that is the point - this is not how the existing CTX functions work > > (ignoring the OPENSSL_CTX stuff). > > Do you have some concrete examples of existing functions that don't work > this way? > SSL_new() BIO_new_ssl()

Re: Reordering new API's that have a libctx, propq

2020-09-14 Thread Matt Caswell
On 14/09/2020 12:46, Tim Hudson wrote: > On Mon, Sep 14, 2020 at 9:19 PM Matt Caswell > wrote: > > I must be misunderstanding your point because I don't follow your logic > at all. > > So this is the correct form according to my proposed policy: > >

Re: Reordering new API's that have a libctx, propq

2020-09-14 Thread Matt Caswell
On 14/09/2020 11:30, Matt Caswell wrote: > In order to try and move this discussion forward I have made a concrete > proposal for how we should formulate the various ideas in this thread > into an actual style. Please see here: > > https://github.com/openssl/web/pull/194 > > Since we're not

Re: Reordering new API's that have a libctx, propq

2020-09-14 Thread Tim Hudson
On Mon, Sep 14, 2020 at 9:19 PM Matt Caswell wrote: > I must be misunderstanding your point because I don't follow your logic > at all. > > So this is the correct form according to my proposed policy: > > TYPE *TYPE_make_from_ctx(char *name,TYPE_CTX *ctx) > And that is the point - this is not

Re: Reordering new API's that have a libctx, propq

2020-09-14 Thread Matt Caswell
On 14/09/2020 11:52, Tim Hudson wrote: > Any proposal needs to deal with the constructors consistently - whether > they come from an OPENSSL_CTX or they come from an existing TYPE_CTX. > That is absent in your PR. > > Basically this leads to the ability to provide inconsistent argument > order

Re: Reordering new API's that have a libctx, propq

2020-09-14 Thread Tim Hudson
Any proposal needs to deal with the constructors consistently - whether they come from an OPENSSL_CTX or they come from an existing TYPE_CTX. That is absent in your PR. Basically this leads to the ability to provide inconsistent argument order in functions. TYPE *TYPE_make_from_ctx(TYPE_CTX

Re: Reordering new API's that have a libctx, propq

2020-09-14 Thread Matt Caswell
In order to try and move this discussion forward I have made a concrete proposal for how we should formulate the various ideas in this thread into an actual style. Please see here: https://github.com/openssl/web/pull/194 Since we're not yet fully in agreement some compromises will have to be

Re: Reordering new API's that have a libctx, propq

2020-09-10 Thread Michael Richardson
Richard Levitte wrote: > There are many red herrings in here, and I would argue that trying to > be too uniform in the way you think about all functions may be > harmful, because not all functions are classified the same. > We cannot deny that many of our interfaces have an OOP

Re: Reordering new API's that have a libctx, propq

2020-09-09 Thread Dr Paul Dale
Still no need for the added complexity: Push: OPENSSL_CTX *prevctx = OPENSSL_CTX_set0_default(libctx); Pop is: OPENSSL_CTX_set0_default(prevctx); Push before callback: OPENSSL_CTX_set0_default(prevctx); Pop after callback: prevctx =

Re: Reordering new API's that have a libctx, propq

2020-09-09 Thread Richard Levitte
On Wed, 09 Sep 2020 16:08:10 +0200, Tomas Mraz wrote: > > On Wed, 2020-09-09 at 22:29 +1000, Dr Paul Dale wrote: > > > On 9 Sep 2020, at 9:38 pm, Tomas Mraz wrote: > > > > > > We could even provide a convenience thread local stack of lib > > > contexts > > > so the caller would not have to keep

Re: Reordering new API's that have a libctx, propq

2020-09-09 Thread Tomas Mraz
On Wed, 2020-09-09 at 22:29 +1000, Dr Paul Dale wrote: > > On 9 Sep 2020, at 9:38 pm, Tomas Mraz wrote: > > > > We could even provide a convenience thread local stack of lib > > contexts > > so the caller would not have to keep the old value but would just > > push > > the new libctx when

Re: Reordering new API's that have a libctx, propq

2020-09-09 Thread Richard Levitte
On Wed, 09 Sep 2020 13:38:42 +0200, Tomas Mraz wrote: > > Regarding model 3, it must be said that there is potential for > > confusion > > on what it's supposed to do, replace the default property query > > string > > (settable with EVP_set_default_properties()), or merge with it. > > Remember

Re: Reordering new API's that have a libctx, propq

2020-09-09 Thread Dr Paul Dale
> On 9 Sep 2020, at 9:38 pm, Tomas Mraz wrote: > > We could even provide a convenience thread local stack of lib contexts > so the caller would not have to keep the old value but would just push > the new libctx when entering and pop the old one when leaving. With > that, I think the changes

Re: Reordering new API's that have a libctx, propq

2020-09-09 Thread Tomas Mraz
On Wed, 2020-09-09 at 11:41 +0200, Richard Levitte wrote: > > Regarding the library context, when viewed as a global state, it > makes > sense to have it as a first argument where it's being passed, if at > all. The question is, where should we actually pass it? We have a > few different

Re: Reordering new API's that have a libctx, propq

2020-09-09 Thread Richard Levitte
A few of the more active developers currently have a videocall meeting every tuesday, in the morning for us in Europe. We talked about this issue yesterday, and realised quite a few things. One pretty important thing to realise is that while many new functions take a library context and a

Re: Reordering new API's that have a libctx, propq

2020-09-07 Thread Richard Levitte
On Sun, 06 Sep 2020 23:40:53 +0200, SHANE LONTIS wrote: > > If it is decided that the libctx is more important then the API > should really be something like this.. > OSSL_LIBCTX_MD_fetch(), (and I dont know if that is a good idea.).. I would rather not see that. -- Richard Levitte

Re: Reordering new API's that have a libctx, propq

2020-09-06 Thread SHANE LONTIS
The example given was EVP_PKEY_get_() and from the API name it is fairly clear what the first param should be EVP_PKEY *pkey (the API tells you this). For the fetch case, I still think that the algorithm is the most important param. Libctx, propq are optional parameters as far as I am

Re: Reordering new API's that have a libctx, propq

2020-09-06 Thread Richard Levitte
There are many red herrings in here, and I would argue that trying to be too uniform in the way you think about all functions may be harmful, because not all functions are classified the same. We cannot deny that many of our interfaces have an OOP flair. We may be programming in C, but we very

Re: Reordering new API's that have a libctx, propq

2020-09-05 Thread Richard Levitte
Hi, so... "importance" quite obviously carries different meaning to different people. What I see below is the meaning "having the longest life span" or possibly "being the biggest and most powerful resource". I've a different interpretation of "importance". Looking at EVP_XXX_fetch(), it's

Re: Reordering new API's that have a libctx, propq

2020-09-05 Thread Nicola Tuveri
On Sat, Sep 5, 2020, 14:01 Tim Hudson wrote: > On Sat, Sep 5, 2020 at 8:45 PM Nicola Tuveri wrote: > >> Or is your point that we are writing in C, all the arguments are >> positional, none is ever really optional, there is no difference between >> passing a `(void*) NULL` or a valid `(TYPE*)

Re: Reordering new API's that have a libctx, propq

2020-09-05 Thread Tim Hudson
On Sat, Sep 5, 2020 at 8:45 PM Nicola Tuveri wrote: > Or is your point that we are writing in C, all the arguments are > positional, none is ever really optional, there is no difference between > passing a `(void*) NULL` or a valid `(TYPE*) ptr` as the value of a `TYPE > *` argument, so

Re: Reordering new API's that have a libctx, propq

2020-09-05 Thread Nicola Tuveri
On Sat, Sep 5, 2020, 12:13 Tim Hudson wrote: > On Sat, Sep 5, 2020 at 6:38 PM Nicola Tuveri wrote: > >> In most (if not all) cases in our functions, both libctx and propquery >> are optional arguments, as we have global defaults for them based on the >> loaded config file. >> As such,

Re: Reordering new API's that have a libctx, propq

2020-09-05 Thread Tim Hudson
On Sat, Sep 5, 2020 at 6:38 PM Nicola Tuveri wrote: > In most (if not all) cases in our functions, both libctx and propquery are > optional arguments, as we have global defaults for them based on the loaded > config file. > As such, explicitly passing non-NULL libctx and propquery, is likely to

Re: Reordering new API's that have a libctx, propq

2020-09-05 Thread Nicola Tuveri
Thanks Tim for the writeup! I tend to agree with Tim's conclusions in general, but I fear the analysis here is missing an important premise that could influence the outcome of our decision. In most (if not all) cases in our functions, both libctx and propquery are optional arguments, as we have

Re: Reordering new API's that have a libctx, propq

2020-09-05 Thread SHANE LONTIS
Thanks for the writeup.. > My view is that the importance of arguments is what sets their order - and > that is why for the TYPE functions the TYPE pointer > comes first. We could have just as easily specified it as last or some other > order, but we didn’t. Isn’t that the order that the fetch

Re: Reordering new API's that have a libctx, propq

2020-09-05 Thread Tim Hudson
I place the OTC hold because I don't believe we should be making parameter reordering decisions without actually having documented what approach we want to take so there is clear guidance. This was the position I expressed in the last face to face OTC meeting - that we need to write such things

RE: Reordering new API's that have a libctx, propq

2020-09-05 Thread Dr. Matthias St. Pierre
ect@openssl.org Subject: Reordering new API's that have a libctx, propq PR #12778 reorders all the API’s of the form: EVP_XX_fetch(libctx, algname, propq) So that the algorithm name appears first.. e.g: EVP_MD_fetch(digestname, libctx, propq); This now logically reads as 'search for this algorithm us

Reordering new API's that have a libctx, propq

2020-09-04 Thread SHANE LONTIS
PR #12778 reorders all the API’s of the form: EVP_XX_fetch(libctx, algname, propq) So that the algorithm name appears first.. e.g: EVP_MD_fetch(digestname, libctx, propq); This now logically reads as 'search for this algorithm using these parameters’. The libctx, propq should always appear