chore(docs): replace IN and OUT macros with docs #1817
Conversation
poiana
added
release-note-none
kind/cleanup
dco-signoff: yes
kind/documentation
|
Please double check driver/API_VERSION file. See versioning. /hold |
Hey @Molter73 I believe this is a false positive. If I understood correctly, your PR is a no-op from the kernelspace<->userspace APIs backward compatibility perspective. Right? |
|
cc @falcosecurity/libs-maintainers for visibility |
Yup, no actual code change should happen since the removed macros expand to empty strings anyways. |
👍 |
|
I don't have any problems with these changes @leogr - let's wait to hear from the others. |
|
it's good for me too. @Molter73 could you rebase pls? 🙏 |
In scap.h, we have a couple of quite generic macros called IN and OUT that can very easily leak into adopter's code bases and wreak havoc. From my understanding, these macros are only used for documenting which parameters in a function are input and which are output. Since they are not consistently used, only appear in a handful of places in the codebase and the preprocessor is the devil's spawn, we should replace these macros with proper doxygen style string docs. Signed-off-by: Mauro Ezequiel Moltrasio <[email protected]> Co-authored-by: Olivier Valentin <[email protected]>
Signed-off-by: Mauro Ezequiel Moltrasio <[email protected]>
Signed-off-by: Mauro Ezequiel Moltrasio <[email protected]>
Molter73
force-pushed
the
remove-in-out-macros
branch
from
6bf09c3 to
16a9ded
Compare
| @@ -199,7 +199,7 @@ void sinsp_evt_formatter::set_format(output_format of, const std::string& fmt) | |||
| j++; | |||
| msize++; // count ')' | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Dang it, I didn't mean to commit whitespace changes 🤦🏻♂️
We really need to get formatting checked by CI.
There was a problem hiding this comment.
I agree.
Anyway, considering the affected file name, the change is on topic 😜
|
LGTM label has been added. Git tree hash: d01e0bf03fdbf9095cb8ef7f5efe03b06be67475
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: incertum, leogr, Molter73 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind cleanup
/kind documentation
Any specific area of the project related to this PR?
/area libscap-engine-bpf
/area libscap-engine-kmod
/area libscap-engine-modern-bpf
/area libscap-engine-source-plugin
/area libscap
/area libsinsp
/area tests
Does this PR require a change in the driver versions?
What this PR does / why we need it:
In scap.h, we have a couple of quite generic macros called IN and OUT that can very easily leak into adopter's code bases and wreak havoc (i.e: we actually had these macros override parameters in an auto-generated enum from protobufs that were literally called
INandOUT, giving very confusing compilation errors).From my understanding, these macros are only used for documenting which parameters in a function are input and which are output. Since they are not consistently used, only appear in a handful of places in the codebase and the preprocessor is the devil's spawn, we should replace these macros with proper doxygen style string docs.
Alternatively, we could rename them to something like
SCAP_INandSCAP_OUT, so collision with adopters code would be harder, but I think removing them is the more sensible approach.Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
This PR touches a bunch of different places in the code base, but it should only be removing the
INandOUTmacros that normally expands to empty strings, so there should be no effective changes to code.The added doc descriptions might not be specific enough, so any comments on how to improve them are more than welcomed.
Does this PR introduce a user-facing change?: