Skip to content

Document ASN1_BIT_STRING functions#30690

Closed
jogme wants to merge 4 commits into
openssl:masterfrom
jogme:asn1_bit_string_docs
Closed

Document ASN1_BIT_STRING functions#30690
jogme wants to merge 4 commits into
openssl:masterfrom
jogme:asn1_bit_string_docs

Conversation

@jogme
Copy link
Copy Markdown
Contributor

@jogme jogme commented Apr 6, 2026

Note: ASN1_BIT_STRING_set() is not mentioned here, because it gets deprecated in a following PR.

Checklist
  • documentation is added or updated
  • tests are added or updated

Signed-off-by: Norbert Pocs <norbertp@openssl.org>
@jogme jogme force-pushed the asn1_bit_string_docs branch from da6a76b to b71846a Compare April 6, 2026 12:58
@openssl-machine openssl-machine added the approval: review pending This pull request needs review by a committer label Apr 6, 2026
@jogme jogme added branch: master Applies to master branch triaged: documentation The issue/pr deals with documentation (errors) labels Apr 6, 2026
@jogme
Copy link
Copy Markdown
Contributor Author

jogme commented Apr 6, 2026

Deprecation PR #30692

esyr
esyr previously approved these changes Apr 8, 2026
Copy link
Copy Markdown
Member

@esyr esyr left a comment

Choose a reason for hiding this comment

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

Can go in as-is, I guess.

Comment thread doc/man3/ASN1_BIT_STRING_get_length.pod Outdated
Comment thread doc/man3/ASN1_BIT_STRING_get_length.pod Outdated
Comment thread doc/man3/ASN1_BIT_STRING_get_length.pod Outdated
Comment thread doc/man3/ASN1_BIT_STRING_get_length.pod Outdated
Comment thread doc/man3/ASN1_BIT_STRING_get_length.pod Outdated


ASN1_BIT_STRING_name_print() prints the corresponding bit name specified in
I<tbl> to I<out> based on the bit string I<bs>. I<indent> might be specified for
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ugh, BIT_STRING_BITNAME is not documented; since ASN1_BIT_STRING_name_print, ASN1_BIT_STRING_num_asc, and ASN1_BIT_STRING_set_asc are the only users, its definition can just be added to SYNOPSIS here.

Also, it has to be described somewhere how tbl is generally interpreted: that it is an array of BIT_STRING_BITNAME structures, bitnum is a bit number (and the array is expected to be sorted in non-descending order on bitnum as the key), lname is a "long"(?), human-readable name for the bit, sname is a "short"(?), standards-defined name, the array is terminated with a { -1, NULL, NULL } sentinel (different functions use different fields for end-of-loop check), the table may contain repeated bitnum values, but expected to do so only for sequential records, with the ones following the first being interpreted as aliases (which are not used in ASN1_BIT_STRING_name_print, but are used for matching in ASN1_BIT_STRING_num_asc, v2i_ASN1_BIT_STRING, and maybe others).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

do you think it's worth to document the tbl? The usage you describe does not look that much consistent

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added the struct to the synopsis, hope it's the format you were thinking of

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Well, it does help, but the currently added/present content leaves the question "what (in what format) should I provide as tbl to ASN1_BIT_STRING_name_print/ASN1_BIT_STRING_num_asc/ASN1_BIT_STRING_set_asc to make them work as I expect" unanswered.

Comment thread doc/man3/ASN1_BIT_STRING_get_length.pod Outdated
Comment thread doc/man3/ASN1_BIT_STRING_get_length.pod
Comment thread doc/man3/ASN1_BIT_STRING_get_length.pod Outdated
Comment thread doc/man3/ASN1_BIT_STRING_new.pod Outdated
Signed-off-by: Norbert Pocs <norbertp@openssl.org>
Signed-off-by: Norbert Pocs <norbertp@openssl.org>
esyr
esyr previously approved these changes Apr 9, 2026
Comment thread doc/man3/ASN1_BIT_STRING_get_length.pod
@t8m t8m added tests: exempted The PR is exempt from requirements for testing branch: 4.0 Applies to openssl-4.0 labels Apr 9, 2026
Signed-off-by: Norbert Pocs <norbertp@openssl.org>
@openssl-machine openssl-machine added approval: done This pull request has the required number of approvals approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: review pending This pull request needs review by a committer approval: done This pull request has the required number of approvals labels Apr 10, 2026
@openssl-machine
Copy link
Copy Markdown
Collaborator

This pull request is ready to merge

openssl-machine pushed a commit that referenced this pull request Apr 15, 2026
Signed-off-by: Norbert Pocs <norbertp@openssl.org>

Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org>
Reviewed-by: Nikola Pajkovsky <nikolap@openssl.org>
MergeDate: Wed Apr 15 12:25:45 2026
(Merged from #30690)
@jogme
Copy link
Copy Markdown
Contributor Author

jogme commented Apr 15, 2026

Merged to master and 4.0. Thank you for the reviews!

@jogme jogme closed this Apr 15, 2026
openssl-machine pushed a commit that referenced this pull request Apr 15, 2026
Signed-off-by: Norbert Pocs <norbertp@openssl.org>

Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org>
Reviewed-by: Nikola Pajkovsky <nikolap@openssl.org>
MergeDate: Wed Apr 15 12:26:36 2026
(Merged from #30690)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Applies to master branch branch: 4.0 Applies to openssl-4.0 tests: exempted The PR is exempt from requirements for testing triaged: documentation The issue/pr deals with documentation (errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants