-
Notifications
You must be signed in to change notification settings - Fork 175
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
adjust pkcs12 example to print out list of certificates found #366
base: master
Are you sure you want to change the base?
Conversation
crypto/pkcs12/pkcs12-example.c
Outdated
if (i != 0 && !(i%16)) printf("\n"); | ||
printf("%02X", certDer[i]); | ||
} | ||
printf("\n"); | ||
XFREE(certDer, NULL, DYNAMIC_TYPE_PKCS); | ||
} | ||
|
||
/* itterate through list if was not passed as null and free each node */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest updating comment to mention display of certs in list as code is no longer just freeing each node
crypto/pkcs12/pkcs12-example.c
Outdated
if (current->buffer != NULL) { | ||
printf("[CERT %d] :", certIdx++); | ||
for (i = 0; i < current->bufferSz; i++) | ||
printf("%02X", current->buffer[i]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The certificate HEX data is printed differently than the data for previous certificates. Specifically, the previous HEX data was displayed in 16-byte rows. The way the certificate list data is currently displayed makes the output look inconsistent, and there might also be some utility to having 16-byte rows, as it's easier to visually parse to a specific location (?).
I'd suggest modifying the certificate list HEX data print logic to match that used for other HEX data. I realize that the [CERT %d] :
text printed prior to each certificate will push the first 16-byte row out of alignment with with subsequent rows... I'd suggest adding a newline to the end of the "CERT" text so the first row of HEX data for each certificate starts at column zero.
crypto/pkcs12/pkcs12-example.c
Outdated
WC_DerCertList* next = current->next; | ||
WC_DerCertList* next; | ||
|
||
next = current->next; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: there's an extra space between next
and =
.
crypto/pkcs12/pkcs12-example.c
Outdated
if (keyDer != NULL) { | ||
printf("HEX of Private Key Read (DER format) :\n"); | ||
for (i = 0; i < keySz; i++) { | ||
if (i != 0 && !(i%16)) printf("\n"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A question: I don't see anything explicitly forbidding use of if
without brackets in the coding standards (and I know you didn't create this code, just moved it)... There is an implicit mention of using "K&R" style and if
as an example... Do we generally allow the "no-bracket" formatting for if
statements? .. maybe only when there isn't an else
statement?
Thanks for the great review! I added a commit to address the feedback. |
if (current->buffer != NULL) { | ||
printf("\n[CERT %d] :", certIdx++); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add newline after cert number: [CERT %d] :\n
... Current output (without new-line) looks like this:
HEX of Certificate LIST (DER format) :
[CERT 0] :308204AA30820392A003020102020900
B7B69033661B6B23300D06092A864886
Adding new-line would align HEX output (looks a little cleaner!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On second thought, don't worry about last change-request, it's trivial and better to have functionality in the product.
No description provided.