The use case is for parameters the receiver doesn't know about. The caller can now know if they were used or not. #9500 raised this.

I agree that a missing critical parameter should raise an error.

Shouldn't functions like OSSL_PARAM_get_int set this flag automatically?

Shouldn't functions like OSSL_PARAM_get_int set this flag automatically?

The locate does currently.
Should the get's do as well? I'm happy to add such.

I've made the get calls set the used flag too.

Thinking about it, the set param calls might want to set the used flag too. This allows the information to flow the other way and avoids problems when the returned string size is zero.

You do realise that OSSL_PARAM can never be const at this point, right?

(I.e this PR is incomplete and should probably be marked WIP)

You do realise that OSSL_PARAM can never be const at this point, right?

I thought that this assumption had died with the mutable return size change.
But yes, I did realise this.

I can see one potential use for const OSSL_PARAM arrays. If we do need it, this suggested change cannot go in. I'm fine with that but we'll still need a way to address the problems #9500 raised and how to create backward compatible ctrl functions (which do check for unknown parameters and return error when they are found). I've also found zero instances in the code thus far but will check more deeply next week -- I ran out of time today for unrelated reasons.

I need to go through the sources for for() loops that strcmp param names still, but the only build failures as a result of this were test cases that initialised the OSSL_PARAM structure directly.

Argh, the get params call are passed a const OSSL_PARAM object :( Hence the Travis failures.

The loss of constness means that there's no promise the receiver won't change anything in a OSSL_PARAM array. This worries me constantly.
Another solution would be to have a separate array of bits that the receiver would set for any used parameter, which would be at the caller's option to pass, or not. They would work like an FD_SET in a select() call. A consequence for this is that we need a param locator that returns an index rather than a pointer...

I do not like the array of bits approach, it requires exposing more publicly. I avoided using bit fields for similar reasons even though they would simplify the structure. None of this is speed or space critical. Let's make it easy for the users.

If the receiver does update other fields in the OSSL_PARAM structure they can expect a crash upon a version change. I have zero problems with this so long as the mutability assumptions are properly documented (which they are currently). This is a cooperative arrangement between the provider and the core.

It we really do want to stop providers from changing other fields, we can make them const. It will cost a few casts in the params.c code but I'd be quite happy to implement this. However, I thought the idea was for the caller/owner to point to their variables and a provider changing the pointers would then be ignored.

That last paragraph makes a good point.

@levitte, which point? That providers are cooperating or that the fields could be const.

Sorry, I meant last sentence

However, I thought the idea was for the caller/owner to point to their variables and a provider changing the pointers would then be ignored.

Okay, thanks. I suspected that it was that one BTW.

Which is preferred:

• back out the param get change or
• make param get calls not accept a const argument?

I don't understand the question. This will typically be useful in calls like EVP_MD_CTX_set_params(), so the caller can check which of the parameters it sent in where actually used, yes? That means the OSSL_PARAM needs to be writable, i.e. signatures like this:

int EVP_MD_CTX_set_params(EVP_MD_CTX *ctx, const OSSL_PARAM params[]);

will have to change to this:

int EVP_MD_CTX_set_params(EVP_MD_CTX *ctx, OSSL_PARAM params[]);

By consequense, OSSL_PARAM_get_int() etc will have to be changed to take a non-const OSSL_PARAM as well.

That answers the question: remove the const.

That answers the question: remove the const.

There was never a question. That is the consequence of this PR, there's no way around that.

Okay, I see what you mean.
I've pushed a commit that removes the const OSSL_PARAM where necessary.

The big question now is does this adequately address the concerns in #9500 (and possible future concerns along similar lines) and is it less evil than the alternatives?

BTW, something I just remembered is that we already have a set of functions that can be used to achieve the intent of this PR; if the providers actually implement the get_param_types / set_param_types kind of calls, the returned constant OSSL_PARAM arrays can be used to check what parameters the implementation handles. In other words, instead of flagging "I used this", the provider must declare "I will use this" in advance.

I prefer that, actually. Could we explore that first? (I'm tempted to -1 here to force that exploration)

@mattcaswell any suggestion on alternative implementations that let us know what's been used? I'm not attached to this PR but I do think we'd benefit from a solution to the underlying issue.

@mattcaswell any suggestion on alternative implementations that let us know what's been used? I'm not attached to this PR but I do think we'd benefit from a solution to the underlying issue.

Perhaps a simple count of the number of parameters that were processed in the return value would be sufficient.

Perhaps a simple count of the number of parameters that were processed in the return value would be sufficient.

That is fine for controls which just send a single value.. Matt could you see this becoming a problem when we start passing multiple params for keys? If it processed 4 out of 5 of the params, what does that really tell us?

Would it be more acceptable to revert the structure and for params.h to subvert some high order bits in the data_type field?

Pretty much everything is using params.h and we could declare the top bits to be implementation dependent.

Just a thought?

That is fine for controls which just send a single value.. Matt could you see this becoming a problem when we start passing multiple params for keys? If it processed 4 out of 5 of the params, what does that really tell us?

I see no problem with that at all. If the caller considers some params mandatory and others to not be, then just perform two calls. One with all the mandatory params, and one with all the optional params.

Either things are opaque between the caller/provider, or they are not. This is be adding a "peek behind the veil" capability, which is wrong. It's deliberately making an abstraction leaky.

If a caller wants a provider to do something, then how it does it should be opaque, and it should return an OK/ERROR status, and pushing a status code on the stack.

I see no problem with that at all. If the caller considers some params mandatory and others to not be, then just perform two calls. One with all the mandatory params, and one with all the optional params.

PQC algorithm barfoo takes arguments foo and bar, one is optional. Which?
One of the points of providers is that the framework doesn't need to know.

Not that this helps here 😢

PQC algorithm barfoo takes arguments foo and bar, one is optional. Which?
One of the points of providers is that the framework doesn't need to know.

Right, the framework doesn't need to know. But applications have to know, there is no way around it. Saying it can be done with zero knowledge of the semantics is magical thinking.

Since this seems to be unwanted, I'll close it soon if no one offers some support.

I see no problem with that at all. If the caller considers some params mandatory and others to not be, then just perform two calls. One with all the mandatory params, and one with all the optional params.

@mattcaswell - I had a look into using counters - it looks error prone. Placing count++ into every 'if locate' block inside every get_ctx_params(), set_ctx_params(), get_params() is not ideal.
Having an array of indexes would also be horrible for the caller side.
Wouldnt it just be much simpler to use param capability flags instead - similar to how it was done in the existing legacy code?

I really like what @levitte proposed. It could be very useful for callers to know in advance what is/can be consumed by the provider. For example various scripting frameworks could adjust their behavior accordingly. I do not think the ex-post knowledge of what was used and what not is that much useful - I mean it could be somewhat useful sometimes to verify that what I passed in was really consumed, but given it complicates the simplicity of the OSSL_PARAM I do not think it is worth it.

If this were flipped around and a flag was set when the param value was changed, would it still feel dirty?

We have this capability currently (except for a zero byte octet string, the return size is set non-zero).

A thought I just had is that this is only a set_params (i.e. OSSL_PARAM_get_xxx() calls) issue, yeah? We want to somehow know what params were actually used, yeah? Wouldn't it be possible to do the same as we do for the other direction, simply set return_size to the size of the parameter data?

A thought I just had is that this is only a set_params (i.e. OSSL_PARAM_get_xxx() calls) issue, yeah? We want to somehow know what params were actually used, yeah? Wouldn't it be possible to do the same as we do for the other direction, simply set return_size to the size of the parameter data?

So the gets should all return an error if the param isnt found?
I think @paulidale has suggested something similiar in his PR?

So the gets should all return an error if the param isnt found?

What? What in what I just wrote gave you the idea I said that?

What? What in what I just wrote gave you the idea I said that?

Err I think I said it :)

We want to somehow know what params were actually used, yeah? Wouldn't it be possible to do the same as we do for the other direction, simply set return_size to the size of the parameter data?

I think that is not that much different from the pill you were having trouble swallowing :)

I think that is not that much different from the pill you were having trouble swallowing :)

Oh hush, you... 😆

I think that part of that pill was how OSSL_PARAM changed an became increasingly complex.
The rest of it... well, we do already set return_size in one direction, I find doing the same in the other direction a much easier pill to swallow, even though I'll mourn the disappearing const

I'm fine with setting the return_size to indicate that the parameter was read, I had thought of this but figured it was abusing the field a little too much. There remains a problem with octet strings of zero length, but I'll forgive and forget that.

Was the pill blue or red?

I'll implement this this week and remove the hold once I've done so.

Was the pill blue or red?

Heh...

Please don't re-use fields. Burn a flag byte.

I'd prefer adding a flag but adding a field to the structure seems to be causing friction.

What I could do is rename return_size to used_size (accessed_size, size or whatever) and set it both in get and set calls.

The parameter stuff is already so complicated that there are multiple/competing PR's to make it easier to create/use/maintain them. And there is friction about adding a byte? People prefer to re-use a field in hacky ways?

I hope the team reconsiders.

Closing this since it isn't sufficiently wanted.
I still think the underlying problem hasn't been well addressed.

I still think the underlying problem hasn't been well addressed.

Agreed