test: port test/test-api.c #3
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hchandad
commented
Sep 12, 2022
|
@hchandad Hi, I tried testing this pr and got a seg fault on this line, the parameter Did you run into this problem before or have any idea why this might be? I will leave the extended backtrace below if it helps. |
|
@StefanJum the above segv occurs when not choosing mimalloc as the default allocator in the boot section. From
Thank you for the detailed info and apologies for not including testing steps, here is what i did: ExpandChanges to app-helloworld Makefile diff --git a/Makefile b/Makefile
index 5426bae..5310f8a 100644
--- a/Makefile
+++ b/Makefile
@@ -1,6 +1,6 @@
UK_ROOT ?= $(PWD)/../../unikraft
UK_LIBS ?= $(PWD)/../../libs
-LIBS :=
+LIBS := $(UK_LIBS)/lib-pthread-embedded:$(UK_LIBS)/lib-newlib:$(UK_LIBS)/lib-mimalloc
all:
@$(MAKE) -C $(UK_ROOT) A=$(PWD) L=$(LIBS)from the menuconfig i set:
I only ported test-api.c, test-stress.c does fail for me when running as app-helloworld entrypoint -without porting it- ( i still haven't figured out how to do backtraces) |
|
@StefanJum actually it is a change to the original sources, but i added it as a separate file, to make it possible to |
StefanJum
suggested changes
Sep 19, 2022
There was a problem hiding this comment.
Great job @hchandad thank you!
I left an initial round of review.
Besides the comments, can you please also run checkpatch on your patch? It looks like there are a few spacing issues.
Also please squash your commits, we ideally want only one commit with a relevant message (see here).
|
@StefanJum I made changes to the coding style, using checkpatch.pl output: |
|
@hchandad you can ignore the errors coming from the original file, just make sure your changes do not produce any more errors. |
StefanJum
reviewed
Sep 22, 2022
|
@StefanJum Thank you, I have updated with the following changes: diff --git a/Config.uk b/Config.uk
index eb3e7a1..05e2c73 100644
--- a/Config.uk
+++ b/Config.uk
@@ -22,13 +22,12 @@ if LIBMIMALLOC
Config LIBMIMALLOC_TES
bool "Enable libmimalloc tests"
select LIBUKTEST
- select LIBMIMALLOC_TEST_API
default n
help
Enables all libmimalloc tests.
if LIBMIMALLOC_TEST
config LIBMIMALLOC_TEST_API
bool "Enable libmimalloc api test suite"
- default n
+ default y
endif endif
diff --git a/Makefile.uk b/Makefile.uk
index f9df74f..0e9fc37 100644
--- a/Makefile.uk
+++ b/Makefile.uk
@@ -97,6 +97,6 @@ LIBMIMALLOC_SRCS-y += $(LIBMIMALLOC)/src/init.c
################################################################################
# Tests
################################################################################
-ifneq ($(filter y, $(CONFIG_LIBMIMALLOC_TEST) $(CONFIG_LIBUKTEST_ALL)),)
+ifneq ($(filter y, $(CONFIG_LIBMIMALLOC_TEST_API) $(CONFIG_LIBUKTEST_ALL)),)
LIBMIMALLOC_SRCS-y += $(LIBMIMALLOC)/test/test-api.c
endif |
Signed-off-by: Hamza Chandad <hchandad@proton.me>
|
@StefanJum fixed. |
razvand
added
enhancement
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related issue : #2
Tested with app-helloworld and 05-app-googletest on plat: kvm-x86_64
Signed-off-by: Hamza Chandad hchandad@proton.me