Skip to content

Use accessors for ASN1_STRING values#676

Merged
simo5 merged 1 commit intoopenssl-projects:mainfrom
bob-beck:main
Feb 12, 2026
Merged

Use accessors for ASN1_STRING values#676
simo5 merged 1 commit intoopenssl-projects:mainfrom
bob-beck:main

Conversation

@bob-beck
Copy link
Contributor

@bob-beck bob-beck commented Jan 30, 2026

OpenSSL is going to make ASN1_STRING opaque. The accessors in question have been around in OpenSSL for a very long time.

see openssl/openssl#29117
and specifically openssl/openssl#29862

Description

Checklist

  • Code modified for feature
  • Test suite updated with functionality tests
  • Test suite updated with negative tests
  • Documentation updated

Reviewer's checklist:

  • Any issues marked for closing are addressed
  • There is a test suite reasonably covering new functionality or modifications
  • This feature/change has adequate documentation added
  • Code conform to coding style that today cannot yet be enforced via the check style test
  • Commits have short titles and sensible commit messages
  • Coverity Scan has run if needed (code PR) and no new defects were found

@bob-beck bob-beck marked this pull request as ready for review January 30, 2026 01:05
Copy link
Collaborator

@simo5 simo5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

almost there :)

@simo5 simo5 added the covscan Triggers Coverity Scanner label Jan 30, 2026
simo5
simo5 previously approved these changes Jan 30, 2026
Copy link
Collaborator

@simo5 simo5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM,
please squash the fixups and add your sign-off to the commit

@github-actions github-actions bot removed the covscan Triggers Coverity Scanner label Jan 30, 2026
@simo5
Copy link
Collaborator

simo5 commented Jan 30, 2026

Covscan servers are down for maintenance, will re-run the scan later, after squash and signoff are applied

Copy link
Contributor Author

@bob-beck bob-beck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@simo5 simo5 added the covscan Triggers Coverity Scanner label Feb 2, 2026
@github-actions github-actions bot removed the covscan Triggers Coverity Scanner label Feb 2, 2026
@simo5
Copy link
Collaborator

simo5 commented Feb 3, 2026

Unfortunately it seems coverity is having some extended maintenance issues, I can waive it off, but the DCO check is required. @bob-beck can you please ass the sign-off-by trailer to your commit ? (git commit -s --amend)

OpenSSL is going to make ASN1_STRING opaque. The accessors
in question have been around in OpenSSL for a very long time.

(see openssl/openssl#29117)

Signed-off-by: Bob Beck <beck@openssl.org>
@bob-beck
Copy link
Contributor Author

Unfortunately it seems coverity is having some extended maintenance issues, I can waive it off, but the DCO check is required. @bob-beck can you please ass the sign-off-by trailer to your commit ? (git commit -s --amend)

ok, I think I have?

@bob-beck
Copy link
Contributor Author

Unfortunately it seems coverity is having some extended maintenance issues, I can waive it off, but the DCO check is required. @bob-beck can you please ass the sign-off-by trailer to your commit ? (git commit -s --amend)

ok, I think I have?

(BTW: I've noticed in my other projects that coverity has been dead for an extended period btw, and we're exploring alternatives at the moment)

@simo5
Copy link
Collaborator

simo5 commented Feb 10, 2026

Unfortunately it seems coverity is having some extended maintenance issues, I can waive it off, but the DCO check is required. @bob-beck can you please ass the sign-off-by trailer to your commit ? (git commit -s --amend)

ok, I think I have?

Yes, re-running tests and will merge

(BTW: I've noticed in my other projects that coverity has been dead for an extended period btw, and we're exploring alternatives at the moment)

Yes, we are looking for alternatives too, apparently Blackduck stopped caring completely and decided to not even communicate a prolong shutdown or service termination, great publicity ...

@simo5 simo5 added the covscan-not-needed Covscan is not needed for this PR label Feb 10, 2026
@simo5 simo5 closed this Feb 11, 2026
@simo5 simo5 reopened this Feb 11, 2026
@simo5
Copy link
Collaborator

simo5 commented Feb 11, 2026

@bob-beck sorry to add more churn, but we just merged the coverity replacement. Would you mind rebasing on main so we can get the csbuild tests running ?

@bob-beck
Copy link
Contributor Author

bob-beck commented Feb 11, 2026 via email

@bob-beck bob-beck requested a review from simo5 February 11, 2026 22:39
@simo5 simo5 merged commit 3fb16b0 into openssl-projects:main Feb 12, 2026
235 of 240 checks passed
@simo5
Copy link
Collaborator

simo5 commented Feb 12, 2026

Thanks for the contribution! (and your patience with CI woes)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

covscan-not-needed Covscan is not needed for this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants