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.

Sign up for GitHub

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

Jump to bottom

Backport 2.16: Query Mbed TLS configuration from ssl_client2 and ssl_server2 #2389

Merged
merged 23 commits into from
Feb 25, 2019
Merged

Backport 2.16: Query Mbed TLS configuration from ssl_client2 and ssl_server2 #2389

merged 23 commits into from
Feb 25, 2019

Conversation

andresag01
Copy link
Contributor

This PR is a backport for #2105

Description

This PR addresses:

The problem was that the detection of the current configuration in ssl-opt.sh used to be based on a fragile combination of calls to grep and sed which did not work in some cases and is not very portable. This PR replaces that mechanism by extending the command line interface for ssl_client2 and ssl_server2 to allow querying the current Mbed TLS configuration. For example, to check if MBEDLTS_AES_C was enabled at compile time, we simply need to run the command:

programs/ssl/ssl_server2 query_config=MBEDTLS_AES_C

A return value of 0 means that the feature is enabled and 1 means it was not. If the macro has a non empty expansion, this will be printed to stdout.

Status

READY

@andresag01 andresag01 added bug mbed TLS team needs-review Every commit must be reviewed by at least two team members, labels Jan 30, 2019
@andresag01 andresag01 mentioned this pull request Jan 30, 2019
Merged
4 tasks
k-stachowiak
k-stachowiak previously approved these changes Jan 31, 2019
View reviewed changes
Copy link
Contributor

@k-stachowiak k-stachowiak left a comment

Choose a reason for hiding this comment

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

I have reviewed the code again, ran the new scripts and make test and ssl-opt.sh tests. They all succeed.

mpg
mpg previously approved these changes Feb 7, 2019
View reviewed changes
Copy link
Contributor

@mpg mpg left a comment

Choose a reason for hiding this comment

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

This looks to me like a faithful backport of the original PR.

@mpg
Copy link
Contributor

mpg commented Feb 7, 2019

@andresag01 This PR now has conflicts in ssl-opt.sh. Please resolve them.

@mpg mpg added needs-work and removed needs-review Every commit must be reviewed by at least two team members, labels Feb 7, 2019
Andres Amaya Garcia and others added 19 commits February 7, 2019 10:30
Query config through ssl_server2 and ssl_client2 cmdline
bfa3e09
ssl-opt: Use ssl_server2 to query config instead of grep and sed
0644678
query_config.c: remove duplicated inc of platform.h
55a6c36
Script generation of query_config.c file
f1a5b26
Add gen_query_config.pl to check-gen-files and bump_version
e144b41
Add ChangeLog entries
d4d5596
Update programs/ssl/CMakeLists.txt with query_config.c dep
aaa6007
Fix typo in quenerate_query_config.pl comment
e8b11dd
Ensure query_config.c includes stdio only when needed
b6ff184
Create programs/test/query_compile_time_config app
ce52886
Fix aligment in programs/test/query_compile_time_config.c
bf83f1d
Fix multiple stdio.h inclusion in query_config.c
289c64a
Fix GCC 0-length printf format string error
d2689e3
Improve comments in query_config.fmt
5547a40
Improve cmake inclusion of query_config.c when building apps
cb6b6dc
Fix missing include in vs proj files for query programs
024694e
Fix query_config macro expansion for windows
c46ea38
Fix query_config macro expansion for windows
c500ad8
Exclude macros from query_config.c generation
4eb040a
@andresag01 andresag01 dismissed stale reviews from mpg and k-stachowiak via 4c47e18 February 7, 2019 10:35
@andresag01 andresag01 force-pushed the mbedtls-2.16-iotssl-2555-config-detection branch from e1d3f46 to 4c47e18 Compare February 7, 2019 10:35
@andresag01
Copy link
Contributor Author

@mpg: Resolved conflicts. Waiting for CI results

@andresag01 andresag01 added needs-review Every commit must be reviewed by at least two team members, and removed needs-work labels Feb 7, 2019
mpg
mpg approved these changes Feb 7, 2019
View reviewed changes
Copy link
Contributor

@mpg mpg left a comment

Choose a reason for hiding this comment

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

Re-approving after rebase

@mpg
Copy link
Contributor

mpg commented Feb 7, 2019

@k-stachowiak Please re-review!

@mpg mpg added approved Design and code approved - may be waiting for CI or backports and removed needs-review Every commit must be reviewed by at least two team members, labels Feb 12, 2019
@mpg
Copy link
Contributor

mpg commented Feb 12, 2019

This has 2 approvals and a passing CI, fixes real bugs, has ChangeLog entries and is well covered by pre-existing tests, so ready for merge.

@Patater Patater merged commit 4c47e18 into Mbed-TLS:mbedtls-2.16 Feb 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mpg mpg mpg approved these changes

@k-stachowiak k-stachowiak k-stachowiak approved these changes

Assignees
No one assigned
Labels
approved Design and code approved - may be waiting for CI or backports bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@andresag01 @mpg @k-stachowiak @Patater