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

Move programs from mbedtls to framework #9939

Open
wants to merge 5 commits into
base: development
Choose a base branch
from

Conversation

Harry-Ramsey
Copy link
Contributor

@Harry-Ramsey Harry-Ramsey commented Jan 29, 2025

Description

Move program files to MbedTLS-Framework. Closes #112.

PR checklist

Please remove the segment/s on either side of the | symbol as appropriate, and add any relevant link/s to the end of the line.
If the provided content is part of the present PR remove the # symbol.

  • changelog provided | not required because:
  • development PR provided # | not required because:
  • framework PR provided Mbed-TLS/mbedtls-framework: #131
  • 3.6 PR provided # | not required because:
  • 2.28 PR provided # | not required because:
  • tests provided | not required because:

Notes for the submitter

Please refer to the contributing guidelines, especially the
checklist for PR contributors.

Help make review efficient:

  • Multiple simple commits
    • please structure your PR into a series of small commits, each of which does one thing
  • Avoid force-push
    • please do not force-push to update your PR - just add new commit(s)
  • See our Guidelines for Contributors for more details about the review process.

This commit moves demo_common.sh and dlopen_demo.sh from MbedTLS to
MbedTLS-framework.

Signed-off-by: Harry Ramsey <harry.ramsey@arm.com>
@Harry-Ramsey Harry-Ramsey self-assigned this Jan 29, 2025
@Harry-Ramsey Harry-Ramsey force-pushed the move-programs-from-mbedtls-to-framework branch 2 times, most recently from 2abee45 to febf47b Compare February 3, 2025 12:49
This commit updates the file paths necessary for dlopen_demo.sh,
metatest.c query_compile_time_config.c, query_config.h,
query_included_headers.c and zeroize.c.

Signed-off-by: Harry Ramsey <harry.ramsey@arm.com>
Signed-off-by: Harry Ramsey <harry.ramsey@arm.com>
This commit updates the include headers for moved program files whilst
also adding the -I flag for compiler to locate the moved headers in
MbedTLS-Framework.

Signed-off-by: Harry Ramsey <harry.ramsey@arm.com>
Signed-off-by: Harry Ramsey <harry.ramsey@arm.com>
@Harry-Ramsey Harry-Ramsey force-pushed the move-programs-from-mbedtls-to-framework branch from febf47b to fab23f5 Compare February 5, 2025 13:40
Copy link
Contributor

@ronald-cron-arm ronald-cron-arm left a comment

Choose a reason for hiding this comment

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

Just some comments about programs/Makefile.

Comment on lines +81 to +83
$(FRAMEWORK)/tests/programs/metatest \
$(FRAMEWORK)/tests/programs/query_compile_time_config \
$(FRAMEWORK)/tests/programs/query_included_headers \
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
$(FRAMEWORK)/tests/programs/metatest \
$(FRAMEWORK)/tests/programs/query_compile_time_config \
$(FRAMEWORK)/tests/programs/query_included_headers \
test/metatest \
test/query_compile_time_config \
test/query_included_headers \

we want to keep the executables in programs/test.

test/selftest \
test/udp_proxy \
test/zeroize \
$(FRAMEWORK)/tests/programs/zeroize \
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
$(FRAMEWORK)/tests/programs/zeroize \
test/zeroize \

ssl/ssl_test_lib.h \
ssl/ssl_test_common_source.c \
$(DEP)

ssl/ssl_test_lib.o: ssl/ssl_test_lib.c ssl/ssl_test_lib.h $(DEP)
echo " CC ssl/ssl_test_lib.c"
$(CC) $(LOCAL_CFLAGS) $(CFLAGS) -c ssl/ssl_test_lib.c -o $@
$(CC) $(LOCAL_CFLAGS) $(CFLAGS) -I$(FRAMEWORK)/tests/programs/ -c ssl/ssl_test_lib.c -o $@
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather add it to LOCAL_CFLAGS. In line 10, something like:
LOCAL_CFLAGS += -I${FRAMEWORK}/tests/programs

ssl/ssl_test_lib.h \
ssl/ssl_test_common_source.c \
$(DEP)

ssl/ssl_test_lib.o: ssl/ssl_test_lib.c ssl/ssl_test_lib.h $(DEP)
echo " CC ssl/ssl_test_lib.c"
$(CC) $(LOCAL_CFLAGS) $(CFLAGS) -c ssl/ssl_test_lib.c -o $@
$(CC) $(LOCAL_CFLAGS) $(CFLAGS) -I$(FRAMEWORK)/tests/programs/ -c ssl/ssl_test_lib.c -o $@
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
$(CC) $(LOCAL_CFLAGS) $(CFLAGS) -I$(FRAMEWORK)/tests/programs/ -c ssl/ssl_test_lib.c -o $@
$(CC) $(LOCAL_CFLAGS) $(CFLAGS) -c ssl/ssl_test_lib.c -o $@

ditto for the other occurences

echo " CC test/metatest.c"
$(CC) $(LOCAL_CFLAGS) $(CFLAGS) -I../library -I../tf-psa-crypto/core test/metatest.c $(LOCAL_LDFLAGS) $(LDFLAGS) -o $@
$(CC) $(LOCAL_CFLAGS) $(CFLAGS) -I../include -I../library -I../tf-psa-crypto/core $(FRAMEWORK)/tests/programs/metatest.c $(LOCAL_LDFLAGS) $(LDFLAGS) -o $@
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
$(CC) $(LOCAL_CFLAGS) $(CFLAGS) -I../include -I../library -I../tf-psa-crypto/core $(FRAMEWORK)/tests/programs/metatest.c $(LOCAL_LDFLAGS) $(LDFLAGS) -o $@
$(CC) $(LOCAL_CFLAGS) $(CFLAGS) -I../library -I../tf-psa-crypto/core $(FRAMEWORK)/tests/programs/metatest.c $(LOCAL_LDFLAGS) $(LDFLAGS) -o $@

the issue with the compilation of metatest was not in this rule but the fact that there was no rule for the target ${FRAMEWORK}/tests/programs/metatest thus an implicit rule (lovely make build system...) was used without the necessary paths to search for the headers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move some files of programs to the framework
2 participants