<div dir="ltr"><div dir="auto"><div><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, Sep 5, 2020, 14:01 Tim Hudson <<a href="mailto:tjh@cryptsoft.com" target="_blank">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 dir="ltr">On Sat, Sep 5, 2020 at 8:45 PM Nicola Tuveri <<a href="mailto:nic.tuv@gmail.com" rel="noreferrer" target="_blank">nic.tuv@gmail.com</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="auto"><div class="gmail_quote" dir="auto"><div dir="ltr" class="gmail_attr">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 "importance" is the only remaining sorting criteria, hence (libctx, propq) are always the most important and should go to the beginning of the args list (with the exception of the `self/this` kind of argument that always goes first)? <br></div></div></div></blockquote><div><br></div><div>That's a reasonable way to express things.</div><div><br></div><div>The actual underlying point I am making is that we should have a rule in place that is documented and that works within the programming language we are using and that over time doesn't turn into a mess.</div><div>We do add parameters (in new function names) and we do like to keep the order of the old function - and ending up with a pile of things in the "middle" is in my view is one of the messes that we should be avoiding.</div></div></div></blockquote></div></div><div dir="auto"><br></div><div dir="auto">We are already adding new functions, with the ex suffix, to allow users to keep using the old version, so given that the users passing to the "_ex" function are already altering their source, why are we limiting us from rationalizing the signature from the PoV of new users and old users alike that are in general quite critic of our API usability, even if it means that applying both "required-ness" and "importance" as sorting criteria sometime we end up adding in the middle? </div><div dir="auto"><br></div><div dir="auto">I don't think I am being dismissive of the needs of existing applications here: if a maintainer is altering their code from using "EVP_foo()" to "EVP_foo_ex()", they will likely also be looking at the documentation of the old and the new API<span class="gmail_default" style="font-family:arial,sans-serif"> versions</span> and there shouldn't be really any additional significanf cost in adding a parameter a the edges or in the middle. </div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>I think the importance argument is the one that helps for setting order and on your "required args" coming first approach the importance argument also applies because it is actually a required argument simply with an implied default - which again puts it not at the end of the argument list. The library context is required - always - there is just a default - and that parameter must be present because we are working in C.<br></div><div></div></div></div></blockquote></div></div><div dir="auto"><br></div><div dir="auto"><font color="#999999">I think I disagree with this, from the API CONSUMER PoV there is a clear difference between a function where they don't need to pass a valid libctx pointer and instead pass NULL (because there is a default associated with passing NULL) and a function like an hypothetical OSSL_LIBCTX_get_this(libctx) or OSSL_LIBCTX_set_that(libctx, value). </font></div><div dir="auto"><font color="#999999"><br></font></div><div dir="auto"><font color="#999999">In the first case the function operates despite the programmer not providing an explicit libctx (because a default one is used), in the latter the programmer is querying/altering directly the libctx and one must be provided. </font></div><div dir="auto"><br></div><div dir="auto"><i><font color="#000000"><span class="gmail_default" style="font-family:arial,sans-serif">…</span> actually only now that I wrote <span class="gmail_default" style="font-family:arial,sans-serif">out </span><span class="gmail_default" style="font-family:arial,sans-serif">the greyed text above</span> I think I am starting to understand what you mean<span class="gmail_default" style="font-family:arial,sans-serif">t</span> all along<span class="gmail_default" style="font-family:arial,sans-serif">!</span></font></i></div><div dir="auto"><br></div><div dir="auto"><div class="gmail_default" style="font-family:arial,sans-serif">Your point is that any API that uses a `libctx` (and `propq`) is always querying/altering a libctx, even if we use NULL as a shortcut to mean "the global one", so if we are fetching an algorithm, getting/setting something on a libctx scope, creating a new object, we are always doing so from a given libctx.</div><div class="gmail_default" style="font-family:arial,sans-serif">In that sense, when an API consumer is passing NULL they are not passing "nothing" (on which my "optional arguments" approach is based), but they are passing NULL as a valid reference to a very specific libctx.</div><div class="gmail_default" style="font-family:arial,sans-serif"><br></div><div class="gmail_default" style="font-family:arial,sans-serif">I now see what you meant, and I can see how it is a relevant thing to make evident also in the function signature/usage.</div><div class="gmail_default" style="font-family:arial,sans-serif"><br></div><div class="gmail_default" style="font-family:arial,sans-serif">But I guess we can also agree that passing NULL as a very specific reference instead of as "nothing", can create a lot of confusion for external consumers of the API, if it took this long for one of us — ­<i>ok, it's me, so probably everyone else understood your point immediately, but still…</i> — to understand the difference you were pointing out, even knowing how these functions work internally and being somewhat familiar with dealing with libctx-s.</div><div class="gmail_default" style="font-family:arial,sans-serif">If we want to convey this difference properly to our users, would it be better to use a new define?<br><br></div><div class="gmail_default" style=""><font face="monospace">#define OSSL_DEFAULT ((void*)42)</font></div><div class="gmail_default" style=""><br></div><div class="gmail_default" style=""><div class="gmail_default" style="font-family:arial,sans-serif">- OSSL_DFLT could be a shorter alternative, if we prefer brevity.</div><div class="gmail_default" style="font-family:arial,sans-serif">- I am intentionally not specifying _LIBCTX as we might reuse a similar mechanism to avoid overloading NULL in other similar situations: e.g. for propq, where NULL is again a shortcut for the global ones, compared with passing "" as an empty properties query, seems like another good candidate for using a symbol to explicitly highlight that the default propq will be used</div><div class="gmail_default" style="font-family:arial,sans-serif"></div></div></div><div dir="auto"><div class="gmail_default" style="font-family:arial,sans-serif">- I would avoid making OSSL_DFLT an alias for NULL to prevent users from sidestepping the define completely and use NULL directly (because "it just works anyway"): that is why I am picking 42 as an example, but any magic number different from NULL/0 and that is most likely/always treated as an invalid userspace memory address would be ok.</div><div class="gmail_default" style="font-family:arial,sans-serif"><br></div><div class="gmail_default" style="font-family:arial,sans-serif">If we had such a define, from the API consumer PoV, libctx stops being an "optional argument" (in the sense I can give it "NULL/nothing" and the function still works) and it is clearly a required argument (passing "NULL/nothing" segfaults/errors) while still retaining the option of having a shortcut symbol to delegate the task of retrieving the default libctx to the called function.</div><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>Whatever it is that we end up decided to do, the rule should be captured and should also allow for the fact that we will evolve APIs and create _ex versions and those two will also evolve and a general rule should be one that doesn't result in an inconsistent treatment for argument order as we add _ex versions.</div></div></div></blockquote><div><br></div><div><div class="gmail_default" style="font-family:arial,sans-serif">If we were not using NULL as an alias for "the default one" rather than for "nothing", the difference between required and simil-optional arguments (those that you can avoid passing to the function by passing "NULL/nothing") would be evident to the users of the API.</div><div class="gmail_default" style="font-family:arial,sans-serif"><br></div><div class="gmail_default" style="font-family:arial,sans-serif">In that case we could agree on sorting arguments as:</div><div class="gmail_default" style="font-family:arial,sans-serif">- required ones (you must pass a valid value, including the special OSSL_DFLT to let the callee retrieve a default value dynamically, or passing NULL/nothing as the output buffer to obtain a length for preallocation)</div><div class="gmail_default" style="font-family:arial,sans-serif">- simil-optional ones (you can pass "NULL/nothing": the function will perform its operations skipping an optional part of it, or using whatever representation of "nothing" as the value of an optional thing, or creating a temporary thing to solve the task at hand in the stead of the one the caller did not pass, etc.)</div><div class="gmail_default" style="font-family:arial,sans-serif">- callback+args</div><div class="gmail_default" style="font-family:arial,sans-serif"><br></div><div class="gmail_default" style="font-family:arial,sans-serif">and sorting by "importance" within each group:</div><div class="gmail_default" style="font-family:arial,sans-serif">- among required args:<br>  * the equivalent of "self/this" for the functions that operate as object methods in a OOP fashion</div><div class="gmail_default" style="font-family:arial,sans-serif">  * libctx (for OOP-like functions, if an object is not libctx-agnostic, there should be very few occasions where both self and libctx appear as arguments, as the libctx should be embedded in the object at creation time if it depends from it, i.e. let's not repeat the EC_POINT/EC_GROUP mess!)</div><div class="gmail_default" style="font-family:arial,sans-serif">  * propquery (always right after libctx, see Note1 below about libctx+propquery)</div><div class="gmail_default" style="font-family:arial,sans-serif">  * functionality-specific required args, in their order of importance (we need to further formalize this, e.g. we have not always been consistent on the order of inputs and outputs, though most of the recommended API are quite consistent in having outputs first, then inputs)</div><div class="gmail_default" style="font-family:arial,sans-serif">- among optional args:</div><div class="gmail_default" style="font-family:arial,sans-serif">  * functionality-specific optional args, in their order of importance</div><div class="gmail_default" style="font-family:arial,sans-serif">  * optional args for auxiliary objects (I would define these as openssl specific things that aid in solving a task, but are not required to solve a task, e.g. BN_CTX, see Note2 below))</div><div class="gmail_default" style="font-family:arial,sans-serif"><br></div><div class="gmail_default" style="font-family:arial,sans-serif"><b>Note1</b>: I am listing propquery separately, although there should be very few cases where we deliberately have libctx as an argument of a function, and no propquery alongside it. If we are querying/altering a libctx, there is a very high chance that we require propquery to fetch something from it; the fact that, in our <b>current</b> implementation/plans, a function taking libctx as an argument does not use/support propquery for accomplishing its task, does not exclude that in the future part of that task might involve fetching another thing from the passed libctx. Given that once it's out in a release that function will stay with us for years to come, we should make extra sure we have a very solid argument covering the current and foreseeable future of that function, even at the cost of having a propquery argument ignored by the current implementation of that function, and marking it down as unused but reserved for future use in the documentation. Note that we currently have functions that take both libctx and propq, but they are not one after the other, eg. `EVP_PKEY_CTX_new_from_pkey(libctx, pkey, propq)`, if we agree that a libctx argument should always come with an attached porpq argument, there is no reason to separate them</div><div class="gmail_default" style="font-family:arial,sans-serif"><br></div><div class="gmail_default" style="font-family:arial,sans-serif"><b>Note2</b>: (expanding on BN_CTX as an optional auxiliary object) In the lifetime of a single high level RSA/DH/DSA/EC operation we have performance benefits from avoiding frequent heap (re)allocations, so we have this openssl specific object as a short-lived stack-like cache, interposing between malloc/realloc/free and the various BN_this() and BN_that() that compute the result of the high level operation. BN_CTX cannot be a global (or libctx-scoped) object, as we want to retain separation between unrelated high level operations and limit the surface for potential side-channel leakage. As a result most of the `BN_` functions that do not operate in-place have an optional `BN_CTX *` argument: the function will use the provided BN_CTX if it is passed, or create a temporary one if "nothing/NULL" is passed. Strictly speaking this BN_CTX object is not a functional requirement, as the implementation could easily replace all the `BN_CTX_get()` calls with conditional usage of either `BN_new()` or `BN_CTX_get()` and with corresponding conditional `BN_free()`, it is our choice to default to creating a fresh `BN_CTX` if the caller passes NULL, to improve code readability and maintainability. From the API consumer PoV, typing `BN_mul(r, a, b, NULL)` means "compute `r = a * b`, with no preexisting BN_CTX" and the implementation might (or might not) create a temporary BN_CTX to use internally or avoid the BN_CTX mechanism altogether.</div><div class="gmail_default" style="font-family:arial,sans-serif"><br></div><div class="gmail_default" style="font-family:arial,sans-serif">What do you (all) think about it?</div><div class="gmail_default" style="font-family:arial,sans-serif"><br></div><div class="gmail_default" style="font-family:arial,sans-serif">Nicola</div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="auto"><div class="gmail_quote" dir="auto"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
</blockquote></div></div>
</blockquote></div></div>
</blockquote></div></div></div>
</div>