Change returned -2 to 0 in EVP_Digest{Sign,Verify}Init()#10815
Change returned -2 to 0 in EVP_Digest{Sign,Verify}Init()#10815levitte wants to merge 1 commit intoopenssl:masterfrom
Conversation
The returned -2 was to mark when these operations are unsupported. However, that breaks away from the previous API and expectations, and there's not enough justification for that not being zero.
levitte
added
branch: master
romen
left a comment
romen
left a comment
There was a problem hiding this comment.
LGTM !
What could we do to put this on the path for revision for a release after 3.0?
I share the wish of uniformity across the board for the family of EVP_<someop>_init() and it seems reasonable to dedicate a return value (-2 is somewhat our default for this in many places) for the "op not supported with this obscure key you gave me".
|
I believe that excluding the change to Do we need a separate PR for that in this case? |
romen
added
approval: done
I don't think so. You've essentially approved a selective backport, and it should be clean. |
I'd really like to be able to do this as well, but having reflected about the broader topic a lot over the past several months, I'm coming around to think that the only reliable way to effect this kind of change is to define completely new APIs with the "proper semantics", and go through the full (5-year) deprecation cycle for the old APIs. The risk of silently breaking applications seems to outweigh the marginal consistency gains, otherwise. |
levitte
added
approval: ready to merge
The returned -2 was to mark when these operations are unsupported. However, that breaks away from the previous API and expectations, and there's not enough justification for that not being zero. Reviewed-by: Nicola Tuveri <[email protected]> Reviewed-by: Tomas Mraz <[email protected]> (Merged from #10815)
|
Merged. 7612366 Change returned -2 to 0 in EVP_Digest{Sign,Verify}Init() The backport to 1.1.1 has a different commit message, so I'm submitting a separate PR for it. |
|
See #10847 for the backport |
4 participants
The returned -2 was to mark when these operations are unsupported.
However, that breaks away from the previous API and expectations, and
there's not enough justification for that not being zero.