Reordering new API's that have a libctx, propq

[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 *ctx, char *name)
or
TYPE *TYPE_make_from_ctx(char *name,TYPE_CTX *ctx)

It simply isn't consistent to basically allow both forms of this approach.

Seeing the OPENSSL_CTX as something different to the other APIs in terms of
its usage when it is providing the context from which something is
constructed is the underlying issue here.
Your PR basically makes rules for "context" arguments which lead to
allowing both the above forms - and making the current usage of CTX values
a different logical order than the OPENSSL_CTX.

Separately, we should have consistency in the naming of the functions which
take an OPENSSL_CTX - the _with_libctx makes no sense now that we settled
on OPENSSL_CTX rather than OPENSSL_LIBCTX or OPENSSL_LIB_CTX as the naming.
We also have a pile of _ex functions that were introduced just to add an
OPENSSL_CTX - those should be consistently named.

The PR should cover that context of how we name the variations of functions
which add the OPENSSL_CTX reference.

The suggested rules for extended functions is inconsistent in stating the
order "should be retained" and then allowing for inserting "at any point".
IHMO either the new function must preserve the existing order and just add
to the end (to easy migration) or it should conform to the naming scheme
and parameter argument order for new functions - one of those should be the
driver rather than basically creating something that is neither - not easy
to migrate to and also not following the documented order. We should be
trying to achieve one of those two objectives.

Tim.


On Mon, Sep 14, 2020 at 8:30 PM Matt Caswell <matt at openssl.org> 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 yet fully in agreement some compromises will have to be
> made. I hope I've come up with something which isn't too abhorrent to
> anyone.
>
> Please take a look.
>
> Matt
>
>
> On 05/09/2020 04:48, SHANE LONTIS wrote:
> >
> >   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 together as a pair of parameters.
> > There are only a few places where only the libctx is needed, which means
> > that if there is no propq it is likely to cause a bug in a fetch at some
> > point.
> >
> > This keeps the API’s more consistent with other existing XXX_with_libctx
> > API’s that put the libctx, propq at the end of the parameter list..
> > The exception to this rule is that callback(s) and their arguments occur
> > after the libctx, propq..
> >
> > e.g:
> > typedef OSSL_STORE_LOADER_CTX *(*OSSL_STORE_open_with_libctx_fn)
> >     (const OSSL_STORE_LOADER *loader,
> >      const char *uri, OPENSSL_CTX *libctx, const char *propq,
> >      const UI_METHOD *ui_method, void *ui_data);
> >
> > An otc_hold was put on this.. Do we need to have a formal vote?
> > This really needs to be sorted out soon so the API’s can be locked down
> > for beta.
> >
> > --------
> > Also discussed in a weekly meeting and numerous PR discussions was the
> > removal of the long winded API’s ending with ‘with_libctx’
> > e.g CMS_data_create_with_libctx
> > The proposed renaming for this was to continue with the _ex notation..
> > If there is an existing _ex name then it becomes _ex2, etc.
> > There will most likely be additional parameters in the future for some
> > API’s, so this notation would be more consistent with current API’s.
> > Does this also need a vote?
> >
> > Regards,
> > Shane
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mta.openssl.org/pipermail/openssl-project/attachments/20200914/74bc60c4/attachment.html>