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

Fix build failure if MBEDTLS_PLATFORM_{CALLOC/FREE}_MACRO are set #2079

Merged
merged 3 commits into from
Nov 8, 2018

Conversation

hanno-becker
Copy link

@hanno-becker hanno-becker commented Oct 11, 2018

Summary: This PR fixes #1642, the issue being a build failure in configurations that use the compile time configuration macros MBEDTLS_PLATFORM_{CALLOC/FREE}_MACRO to set custom calloc() and free() functions. If these macros are defined, the API for runtime configurations mbedtls_platform_set_calloc_free() should be omitted from platform.c (as in platform.h) but it's not. For the development branch, this leads to compilation warning (hence failure in some build modes) about function definition without prior declaration, while in the LTS branches Mbed TLS 2.1 and Mbed TLS 2.7, the compilation fails due to multiple function definitions (the reason for the different symptoms is commit 7decfe8 was applied in between 2.7 and the development branch).

Internal Reference: IOTSSL-2312, IOTSSL-2358.

Fixes #882 fixes #1642 fixes #1706.

Hanno Becker added 2 commits October 11, 2018 11:04
This commit adds a test to tests/scripts/all.sh exercising an
ASan build of the default configuration with

MBEDTLS_PLATFORM_MEMORY enabled,
MBEDTLS_PLATFORM_CALLOC_MACRO set to std calloc
MBEDTLS_PLATFORM_FREE_MACRO   set to std free

(This should functionally be indistinguishable from a default build)
This commit removes the definition of the API function

`mbedtls_platform_set_calloc_free()`

from `library/platform.c` in case the macros

`MBEDTLS_PLATFORM_CALLOC_MACRO`
`MBEDTLS_PLATFORM_FREE_MACRO`

for compile time configuration of calloc/free are set.

This is in line with the corresponding header `mbedtls/platform.h`
which declares `mbedtls_platform_set_calloc_free()` only if
`MBEDTLS_PLATFORM_{CALLOC/FREE}_MACRO` are not defined.

Fixes Mbed-TLS#1642.
mpg
mpg previously approved these changes Oct 16, 2018
@hanno-becker hanno-becker requested review from k-stachowiak and removed request for RonEld October 19, 2018 10:13
k-stachowiak
k-stachowiak previously approved these changes Oct 19, 2018
@hanno-becker hanno-becker removed the needs-review Every commit must be reviewed by at least two team members, label Oct 19, 2018
ChangeLog Outdated Show resolved Hide resolved
@hanno-becker
Copy link
Author

hanno-becker commented Oct 25, 2018

@k-stachowiak @mpg @RonEld Please re-review the amendment of the ChangeLog.

ChangeLog Show resolved Hide resolved
@hanno-becker hanno-becker requested a review from RonEld October 25, 2018 14:45
@RonEld
Copy link
Contributor

RonEld commented Oct 25, 2018

Please fix the backports as well, as they are currently already marked as ready for merge

@RonEld RonEld added the needs-backports Backports are missing or are pending review and approval. label Oct 25, 2018
@hanno-becker
Copy link
Author

@RonEld Done.

@mpg
Copy link
Contributor

mpg commented Oct 29, 2018

Note: I edited the PR description to add that it #882 is fixed too (according to the ChangeLog) and correct the markup so that github understands it: it's "fixes xxx fixes yyy fixes zzz", not "fixes xxx, yyy and zzz".

@mpg mpg removed the needs-backports Backports are missing or are pending review and approval. label Oct 29, 2018
@simonbutcher simonbutcher added the approved Design and code approved - may be waiting for CI or backports label Nov 4, 2018
@simonbutcher simonbutcher merged commit e4f965d into Mbed-TLS:development Nov 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Design and code approved - may be waiting for CI or backports bug component-tls
Projects
None yet
5 participants