Skip to content

Clarify test descriptions in test_suite_memory_buffer_alloc #310

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

Conversation

gilles-peskine-arm
Copy link
Collaborator

@gilles-peskine-arm gilles-peskine-arm commented Oct 31, 2019

One test case prints "FATAL: verification of first header failed". This is actually expected because the test case deliberately sets up a heap that can't have a valid first header. Clarify that in the test description and in the code. Fix #309.

I also removed some spurious dependencies on MBEDTLS_MEMORY_DEBUG in that test suite. I'm not touching the test scripts: that's out of scope and memory_buffer_alloc is only minimally maintained so doesn't deserve extra testing.

Should be backported to Mbed TLS LTS branches.

The test case "Memory buffer small buffer" emits a message
"FATAL: verification of first header failed". In this test case, it's
actually expected, but it looks weird to see this message from a
passing test. Add a comment that states this explicitly, and modify
the test description to indicate that the failure is expected, and
change the test function name to be more accurate.

Fix ARMmbed#309
None of the test cases in tests_suite_memory_buffer_alloc actually
need MBEDTLS_MEMORY_DEBUG. Some have additional checks when
MBEDTLS_MEMORY_DEBUG but all are useful even without it. So enable
them all and #ifdef out the parts that require DEBUG.
@gilles-peskine-arm gilles-peskine-arm added enhancement New feature or request needs: review The pull request is ready for review. This generally means that it has no known issues. labels Oct 31, 2019
@AndrzejKurek AndrzejKurek self-requested a review November 4, 2019 13:06
@gilles-peskine-arm gilles-peskine-arm added needs: backports Needs backports to Mbed TLS branches needs: ci Needs a passing full CI run and removed needs: review The pull request is ready for review. This generally means that it has no known issues. labels Nov 26, 2019
@gilles-peskine-arm
Copy link
Collaborator Author

@gilles-peskine-arm
Copy link
Collaborator Author

Backports: Mbed-TLS/mbedtls#2936 Mbed-TLS/mbedtls#2937

@gilles-peskine-arm gilles-peskine-arm merged commit b951fd9 into ARMmbed:development Nov 26, 2019
gilles-peskine-arm added a commit to gilles-peskine-arm/mbed-crypto that referenced this pull request Feb 3, 2020
* ARMmbed#321: Replace config.pl by config.py
* ARMmbed#322: Update Mbed Crypto with latest Mbed TLS changes as of 2019-11-15
* ARMmbed#308: Small performance improvement of mbedtls_mpi_div_mpi()
* ARMmbed#324: test_psa_constant_names: support key agreement, better code structure
* ARMmbed#320: Link to the PSA crypto portal page from README.md
* ARMmbed#293: Always gather MBEDTLS_ENTROPY_BLOCK_SIZE bytes of entropy
* ARMmbed#310: Clarify test descriptions in test_suite_memory_buffer_alloc
* ARMmbed#307: Add ASN.1 ENUMERATED tag support
* ARMmbed#328: Remove dependency of crypto_values.h on crypto_extra.h
* ARMmbed#325: Rename psa_asymmetric_{sign_verify} to psa_{sign,verify}_hash

Missed listing in the previous submodule update:

* ARMmbed#304: Make sure Asan failures are detected in 'make test'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs: backports Needs backports to Mbed TLS branches
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test_suite_memory_buffer_alloc says "FATAL: verification of first header failed" but passes
3 participants