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

Run clang-format and clang-tidy on eBPF code #6669

Merged
merged 11 commits into from
Mar 4, 2021

Conversation

brycekahle
Copy link
Member

@brycekahle brycekahle commented Oct 30, 2020

What does this PR do?

Run clang-format and clang-tidy on eBPF code. It runs automatically during the system-probe.test target. Issues do not fail test runs as the moment. This will allow us to gradually fix issues highlighted, rather than forcing all development to stop.

Incorporating these checks into a pre-commit hook is ideal, but is complicated by the need to have path-dependent compiler flags. I'm leaving that to a later PR.

Motivation

Ensure ebpf C code is following a consistent style and format.

Additional Notes

We cannot run clang-tidy on the BCC code, since it doesn't compile with clang.

@brycekahle brycekahle added changelog/no-changelog team/networks [deprecated] qa/skip-qa - use other qa/ labels [DEPRECATED] Please use qa/done or qa/no-code-change to skip creating a QA card labels Oct 30, 2020
@brycekahle brycekahle added this to the 7.25.0 milestone Oct 30, 2020
@brycekahle brycekahle requested review from a team as code owners October 30, 2020 20:34
@brycekahle brycekahle force-pushed the bryce.kahle/ebpf-clang-tidy branch from 4611dee to a203291 Compare October 30, 2020 20:36
@albertvaka
Copy link
Contributor

Don't know why but the tests are failing with:

./pkg/ebpf/c/bpf_helpers.h:4:10: fatal error: 'uapi/linux/bpf.h' file not found
#include <uapi/linux/bpf.h>

@brycekahle brycekahle force-pushed the bryce.kahle/ebpf-clang-tidy branch from e43f6c8 to e028006 Compare November 2, 2020 17:29
@mx-psi
Copy link
Member

mx-psi commented Nov 3, 2020

Just a comment: It's not necessary/desirable to tackle this on this PR but we have pre-commit hooks where we could also add this so that people can easily check this client-side https://github.com/DataDog/datadog-agent/blob/master/.pre-commit-config.yaml

pkg/ebpf/c/tracer-ebpf.c Outdated Show resolved Hide resolved
@brycekahle brycekahle force-pushed the bryce.kahle/ebpf-clang-tidy branch from e028006 to 5f51963 Compare November 6, 2020 19:10
@brycekahle brycekahle marked this pull request as draft November 13, 2020 21:32
@brycekahle brycekahle force-pushed the bryce.kahle/ebpf-clang-tidy branch from 5f51963 to 52d62a9 Compare November 20, 2020 19:08
@brycekahle brycekahle modified the milestones: 7.25.0, 7.26.0 Dec 11, 2020
@brycekahle brycekahle modified the milestones: 7.26.0, 7.27.0 Jan 22, 2021
@brycekahle brycekahle force-pushed the bryce.kahle/ebpf-clang-tidy branch from 52d62a9 to 319a8b9 Compare March 2, 2021 22:34
@brycekahle brycekahle marked this pull request as ready for review March 2, 2021 22:47
@brycekahle brycekahle force-pushed the bryce.kahle/ebpf-clang-tidy branch from 6432124 to eb81743 Compare March 4, 2021 14:39
@brycekahle brycekahle merged commit 7eb00d1 into master Mar 4, 2021
@brycekahle brycekahle deleted the bryce.kahle/ebpf-clang-tidy branch March 4, 2021 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog/no-changelog [deprecated] qa/skip-qa - use other qa/ labels [DEPRECATED] Please use qa/done or qa/no-code-change to skip creating a QA card team/networks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants