Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

chore(ci): run static/dynamic analysis #112

Merged
merged 1 commit into from
Oct 24, 2022
Merged

Conversation

mayeut
Copy link
Contributor

@mayeut mayeut commented Oct 23, 2022

This PR updates the workflow to run some static / dynamic analysis before running all other jobs.
It updates the unit test in order to get a reproducer for #111

MACHINE=$(uname -m)
uname -a
gcc --version
clang --version
Copy link
Owner

Choose a reason for hiding this comment

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

${CC} --version, no? The rest of the script is compiler agnostic.

Copy link
Contributor Author

@mayeut mayeut Oct 24, 2022

Choose a reason for hiding this comment

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

clang is used explicitly in make analyze
I changed gcc by ${CC} and explicitly export CC=gcc

make clean
make
make -C test valgrind
echo "::endgroup::"
Copy link
Owner

Choose a reason for hiding this comment

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

There's a lot of repetition in this file. I'd be tempted to avoid the copypasting and use a shell loop to enable/disable inline asm:

for ASM_ENABLED in 0 1; do
    ...
done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes indeed

continue;
}
memcpy(src, &chr[i], chrlen);
}
Copy link
Owner

Choose a reason for hiding this comment

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

Nice, and very useful.

@aklomp aklomp merged commit 5e5f63f into aklomp:master Oct 24, 2022
@mayeut mayeut deleted the valgrind branch October 24, 2022 21:16
@aklomp aklomp mentioned this pull request Nov 8, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants