<div dir="auto"><div dir="auto">Thanks Tim for the writeup! </div><div dir="auto"><br></div><div dir="auto"><br></div>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.<div dir="auto"><br></div><div dir="auto">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.</div><div dir="auto">As such, explicitly passing non-NULL libctx and propquery, is likely to be an exceptional occurrence rather than the norm.</div><div dir="auto"><br></div><div dir="auto">For optional parameters most developers from C and a variety of languages, would expect them to come at the end of the list of parameters, and this also follows the rule of thumb of "importance" used by Tim to pick 1) and B/A). </div><div dir="auto"><br></div><div dir="auto">For this reason I would instead suggest to go with 2) and C) in this case (with the caveat of keeping callback and its args as the very last arguments, again this is a non-written convention not only for us but quite widespread). </div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto">Nicola</div><div dir="auto"><div dir="auto"><br></div><br><br><div class="gmail_quote" dir="auto"><div dir="ltr" class="gmail_attr">On Sat, Sep 5, 2020, 10:10 Tim Hudson <<a href="mailto:tjh@cryptsoft.com">tjh@cryptsoft.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>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.</div><div>This was the position I expressed in the last face to face OTC meeting - that we need to write such things down - so that we avoid this precise situation - where we have new functions that are added that introduce the inconsistency that has been noted here that PR#12778 is attempting to address.</div><div><br></div><div>However, this is a general issue and not a specific one to OPENSSL_CTX and it should be discussed in the broader context and not just be a last minute (before beta1) API argument reordering.</div><div>That does not provide anyone with sufficient time to consider whether or not the renaming makes sense in the broader context.</div><div>I also think that things like API argument reordering should have been discussed on openssl-project so that the broader OpenSSL community has an opportunity to express their views.</div><div dir="ltr"><br></div><div dir="ltr">Below is a quick write up on APIs in an attempt to make it easier to hold an email discussion about the alternatives and implications of the various alternatives over time.</div><div dir="ltr">I've tried to outline the different options.<br><br>In general, the OpenSSL API approach is of the following form:<br><br><i>rettype  TYPE_get_something(TYPE *,[args])<br>rettype  TYPE_do_something(TYPE *,[args])<br>TYPE    *TYPE_new([args])</i><br><br>This isn't universally true, but it is the case for the majority of OpenSSL functions.<br><br>In general, the majority of the APIs place the "important" parameters first, and the ancillary information afterwards.<br><br>In general, for output parameters we tend to have those as the return value of the function or an output parameter that<br>tends to be at the end of the argument list. This is less consistent in the OpenSSL APIs.<br><br>We also have functions which operate on "global" information where the information used or updated or changed<br>is not explicitly provided as an API parameter - e.g. all the byname functions.<br><br>Adding the OPENSSL_CTX is basically providing where to get items from that used to be "global".<br>When performing a lookup, the query string is a parameter to modify the lookup being performed.<br><br>OPENSSL_CTX is a little different, as we are adding in effectively an explicit parameter where there was an implicit (global)<br>usage in place. But the concept of adding parameters to functions over time is one that we should have a policy for IMHO.<br><br>For many APIs we basically need the ability to add the OPENSSL_CTX that is used to the constructor so that <br>it can be used for handling what used to be "global" information where such things need the ability to <br>work with other-than-the-default OPENSSL_CTX (i.e. not the previous single "global" usage).<br><br>That usage works without a query string - as it isn't a lookup as such - so there is no modifier present.<br>For that form of API usage we have three choices as to where we place things:<br><br>1) place the context first<br><br><i>TYPE     *TYPE_new(OPENSSL_CTX *,[args])</i><br><br>2) place the context last<br><br><i>TYPE     *TYPE_new([args],OPENSSL_CTX *)</i><br><br>3) place the context neither first nor last<br><br><i>TYPE     *TYPE_new([some-args],OPENSSL_CTX *,[more-args])</i><br><br>Option 3 really isn't a sensible choice to make IMHO.<br><br>When we are performing effectively a lookup that needs a query string, we have a range of options.<br>If we basically state that for a given type, you must use the OPENSSL_CTX you have been provided with on construction,<br>(not an unreasonable position to take), then you don't need to modify the existing APIs.<br><br>If we want to allow for a different OPENSSL_CTX for a specific existing function, then we have to add items.<br>Again we have a range of choices:<br><br>A) place the new arguments first<br><br><i>rettype  TYPE_get_something(OPENSSL_CTX *,TYPE *,[args])<br>rettype  TYPE_do_something(OPENSSL_CTX *,TYPE *,[args])</i><br><br>B) place the new arguments first after the TPYE<br><br><i>rettype  TYPE_get_something(TYPE *,OPENSSL_CTX *,[args])<br>rettype  TYPE_do_something(TYPE *,OPENSSL_CTX *,[args])<br></i><br>C) place the new arguments last <br><i><br>rettype  TYPE_get_something(TYPE *,[args], OPENSSL_CTX *)<br>rettype  TYPE_do_something(TYPE *,[args], OPENSSL_CTX *)</i><br><br>D) place the new arguments neither first nor last<br><br><i>rettype  TYPE_get_something(TYPE *,[some-args], OPENSSL_CTX *,[more-args])<br>rettype  TYPE_do_something(OPENSSL_CTX *,TYPE *,[some-args], OPENSSL_CTX *,[more-args])<br></i><br>Option D really isn't a sensible choice to make IMHO.<br><br>My view is that the importance of arguments is what sets their order - and that is why for the TYPE functions the TYPE pointer<br>comes first. We could have just as easily specified it as last or some other order, but we didn't.<br><br>Now when we need to add a different location from which to retrieve other information we need to determine where this gets added.<br>I'd argue that it is at constructor time that we should be adding any OPENSSL_CTX or query parameter for any existing TYPE usage<br>in OpenSSL. If we feel the need to cross OPENSSL_CTX's (logically that is what is being done) inside the context of a single instance<br>then we would have to make a choice as to where to place these arguments.<br><br>A very simple thing to note is that we will add arguments over time and that any time we add arguments, applications have to change<br>their code. We basically have again a similar set of choices as to where new arguments get added (first, middle, last). If we make <br>a choice to decide our scheme is to add to the end of the list, then effectively we are choosing to place things in the middle <br>as over time that is what the API pattern becomes when new arguments are added logically. This is of course ignoring that we<br>create a new function when we are adding arguments - but they can conceptually be seen as the same function which we only added<br>because we are coding in C and don't have polymorphic function support.<br><br>I believe the right answer for how we should approach things is 1) and B) (with A as a fallback position).</div><div dir="ltr"><br></div><div dir="ltr"><i>TYPE     *TYPE_new(OPENSSL_CTX *,[args])</i><br></div><div dir="ltr"><i>rettype  TYPE_get_something(TYPE *,OPENSSL_CTX *,[args])<br>rettype  TYPE_do_something(TYPE *,OPENSSL_CTX *,[args])  <br></i></div><div dir="ltr"><br>I think 2) and C) logically turn into 3) and D) over time and that 3) and D) are undesirable outcomes.</div><div dir="ltr"><br></div><div>I also think the pattern works equally well with the query string or any other parameter being included.</div><div><br></div><div><div dir="ltr"><i>TYPE     *TYPE_new(OPENSSL_CTX *,char *prop_query,[args])<br></i></div><div dir="ltr"><i>rettype  TYPE_get_something(TYPE *,OPENSSL_CTX *,char *prop_query,[args])</i></div><div dir="ltr"><i>rettype  TYPE_do_something(TYPE *,OPENSSL_CTX *,char *prop_query,[args]) </i></div></div><div dir="ltr"><br></div><div dir="ltr">Tim.<br></div><div><br></div></div>
</blockquote></div></div></div>