Looks like it needs a rebase..

Ping

Should it be done via ERR_set_mark ERR_pop_to_mark? Or there cannot be any relevant error in the stack before the calls?

What's up wish the fetisch of old stale errors that are ignored? I don't get that.

In practice, though, what caused me to add this PR was that evp_test ran into cases where it couldn't see errors it expected at the start of the error queue because there were ignored errors before that.

What's up wish the fetisch of old stale errors that are ignored? I don't get that.

In practice, though, what caused me to add this PR was that evp_test ran into cases where it couldn't see errors it expected at the start of the error queue because there were ignored errors before that.

Isn't the second sentence an answer to the first one? Shouldn't a test checking errors on the stack fail if any of the functions called in the test raises an error unexpected by the unit test? If we clear the whole stack, rather than just up to what is relevant for the function clearing errors, you are masking potential errors from function calls before the current one!

At that point, @romen, I take it that any previous error is unrelated and thereby ignored.

I think that there's a fundamental difference in the view of what the error queue is. I see it as a stack of current errors, i.e if the caller gets an indication of an error having happened (return value NULL, 0 or negative), the error queue holds information on errors that led to that error return and not stuff that are related to a previous error which has already been handled.
If the common view is that the error queue is a log, which isn't quite the same thing, then I can understand wanting to preserve earlier entries. But in that case, a ring implementation looks odd to me.

@t-j-h, can you remind us what the intent was with the error queue design? You are the better historian 😉

I understand the error queue is not a log. I am more concerned about the situation where such libcrypto calls where we invoke ERR_clear_error() are called internally from other libcrypto calls which could have produced some unhandled error before. Then these unhandled errors are lost by the clear call. If we are sure that in these concrete cases this cannot happen, I am OK with ERR_clear_error() calls.

Right now, I would only find this kind of sequence justifiable:

    ERR_set_mark(); /* to clear collection if none of them succeeds */
    if (try_this()
        || try_that()
        || try_harder())
        ERR_pop_to_mark();
    else
        ERR_clear_last_mark();

In other words, that's code where you do want to collect all the errors that happened down the call path, unless one of them succeeds, at which point none of the errors they generated is interesting any more.

This PR doesn't have that sort of situation in mind.

What if any of the functions you are proposing to use ERR_clear_error is called inside any of the try_this, try_that or try_harder functions?

Also, if all of the three attempts in your example fail, would we really clear the last mark or would we probably admit defeat and return a failure value to the caller and wish to leave on the error stack all the possible things that are wrong with handling the item that makes us fail no matter how hard we try?

What if any of the functions you are proposing to use ERR_clear_error is called inside any of the try_this, try_that or try_harder functions?

....
Hah. You got me there. OK, this convinces me.

As for ERR_clear_last_mark(), what do you think it does?

@levitte

As for ERR_clear_last_mark(), what do you think it does?

Good point! I think I should post less and look more at the code when I am unwell 😂

So I made a few changes, please have a look

The original concepts in the error stack (and it was always seen as a stack of errors) is that communication of the context of an error is one of the more interesting problems to tackle. You need to be able to add additional information and different levels and different places to be able to provide details to the user as to where and how things went wrong.

It wasn't see as a stack trace back mechanism - although it can provide effectively the same thing - as it isn't about a specific call chain - it is about collecting error status related to a set of activities.

There was very little use of internal ERR_clear_error within the library itself as it was up to the application to indicate it wanted a "fresh" start in collecting errors - so a few key points did the reset (like start of connection for client/server, start of verification, PEM processing).

Thanks, @t-j-h. That was very valuable to know.

Ping

Unfortunately, a bit of a rewrite was needed to adapt to other changes in the same files. It was complex enough that a re-approval is needed.

Looks like this one is already "ready to merge"

Right.

Merged.

0b9dd38 EVP: clear error when falling back from failed EVP_KEYMGMT_fetch()
7b131de PROV: Add support for error queue marks and implement in FIPS module