Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move 3 functions to the appropriate util module. #34

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

martin-barta-sie
Copy link
Contributor

No description provided.

Copy link
Member

@DDvO DDvO left a comment

Choose a reason for hiding this comment

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

Rather than moving the three functions, please just rename them.

* @return ASN.1 representation of the DN, or null on error
*******************************************************************************/
/* this function is used by the genCMPClient API implementation */
X509_NAME* UTIL_parse_name(const char* dn, long chtype, bool multirdn);
Copy link
Member

Choose a reason for hiding this comment

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

I acknowledge the discrepancy between the name and the location of the function,
but better not move it to UTIL, which is meant just for stuff that does not fit better elsewhere.
Instead, just rename the function to include CERT rather than UTIL_.

* @param no_duplicate flag governing whether to add cert if it is a duplicate
* @return true on success, else false
*******************************************************************************/
bool UTIL_sk_X509_add1_cert(STACK_OF(X509) * sk, X509* cert, bool no_duplicate);
Copy link
Member

Choose a reason for hiding this comment

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

I acknowledge the discrepancy between the name and the location of the function,
but better not move it to UTIL, which is meant just for stuff that does not fit better elsewhere.
Instead, just rename the function to include CERT rather than UTIL_.

* @return true on success, else false
*******************************************************************************/
/* this function is used by the genCMPClient API implementation */
int UTIL_sk_X509_add1_certs(STACK_OF(X509) * sk, OPTIONAL const STACK_OF(X509) * certs, int no_self_signed,
Copy link
Member

Choose a reason for hiding this comment

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

I acknowledge the discrepancy between the name and the location of the function,
but better not move it to UTIL, which is meant just for stuff that does not fit better elsewhere.
Instead, just rename the function to include CERT rather than UTIL_.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants