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

Remove OSSLzu macros and use %zu (via BIO_printf). #3730

Closed
wants to merge 1 commit into from

Conversation

paulidale
Copy link
Contributor

Also convert the debug prints in dtls_mtu_test.c to use the output framework.

  • tests are added or updated

@paulidale paulidale mentioned this pull request Jun 20, 2017
1 task
@richsalz richsalz self-assigned this Jun 21, 2017
@richsalz richsalz added the approval: done This pull request has the required number of approvals label Jun 21, 2017
@richsalz
Copy link
Contributor

please look at #3727 -- should I use %z in my BIO_printf calls?

@paulidale
Copy link
Contributor Author

@richsalz you are printing uint64_t variables. %zu is for size_t. So no, don't change to %z it would be wrong when size_t is not eight bytes.

@kroeckx
Copy link
Member

kroeckx commented Jun 21, 2017

We have that macro because %z is not part of the C90 standard.

@paulidale
Copy link
Contributor Author

Does BIO_printf always support %zu ?
It's this function being used here.

@t-j-h
Copy link
Member

t-j-h commented Jun 21, 2017

Understood @kroeckx - but it is supported by BIO_printf which is what @paulidale is changing things over to use.

@kroeckx
Copy link
Member

kroeckx commented Jun 21, 2017

I guess the commit message is misleading

Convert the debug prints in dtls_mtu_test.c to use the framework.
@paulidale paulidale changed the title Remove OSSLzu macros and use %zu directly. Remove OSSLzu macros and use %zu directly (via BIO_printf). Jun 21, 2017
@paulidale paulidale changed the title Remove OSSLzu macros and use %zu directly (via BIO_printf). Remove OSSLzu macros and use %zu (via BIO_printf). Jun 21, 2017
@paulidale
Copy link
Contributor Author

I've changed the commit message and the PR title to indicate that BIO_printf is being used.

@dot-asm
Copy link
Contributor

dot-asm commented Jun 21, 2017

Does BIO_printf always support %zu ?

In master, yes.

@richsalz
Copy link
Contributor

Should we add %q or something similar to support printing 64bit values? (Yes, a separate PR)

@richsalz
Copy link
Contributor

merged. next time do your own, kiddo :) once you can, that is.

@richsalz richsalz closed this Jun 21, 2017
levitte pushed a commit that referenced this pull request Jun 21, 2017
Convert the debug prints in dtls_mtu_test.c to use the framework.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from #3730)
@paulidale
Copy link
Contributor Author

I'm still fighting the local network restrictions :(

@paulidale
Copy link
Contributor Author

Thanks though.

@paulidale paulidale deleted the tls13encryption branch June 22, 2017 00:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: done This pull request has the required number of approvals
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants