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

feat(HIS): add CALLS his metric #59

Open
wants to merge 3 commits into
base: wip/his-metrics
Choose a base branch
from

Conversation

AfonsoSantos96
Copy link
Member

@AfonsoSantos96 AfonsoSantos96 commented Oct 10, 2023

This PR introduces the feature of checking the number of function calls inside a function.
Each function must only call a maximum of other 7 functions during its execution flow.
The cflow tool is used to check this metric.

Checklist:

  • The changes generate no new warnings when building the project. If so, I have justified above.
  • I have run the CI checkers before submitting the PR to avoid unnecessary runs of the workflow.

Copy link
Member

@danielRep danielRep left a comment

Choose a reason for hiding this comment

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

While testing this inside the partitioner, I'm getting this log output:

== Checking McCabe Cyclomatic Complexity metric ==
At src/core/main.c(4): complex_function has a complexity of 12. The maximum is 10
"/home/daniel/workspace/osyxtech/bao-partitioner/src/arch/armv8/inc/sysregs.h", line 83: expected pretty much anything after closing paren of function, but got EOF

== Check done with 1 error(s) ==

== Checking the number of called functions ==
At /core/main.c (81): calling_function calls 8 functions.The maximum are 7 calls
cflow:/home/daniel/workspace/osyxtech/bao-partitioner/src/core/console.c:37: chars_writen redefined
cflow:/home/daniel/workspace/osyxtech/bao-partitioner/src/core/console.c:33: this is the place of previous definition
== Check done with 1 error(s) ==

make: *** [ci/ci.mk:299: his-check] Error 255

The first report seems correct, since I introduce those +7 function calls.
The next two reports regarding console.c seem that the tool is checking other properties outside the scope of the his metric introduced in this PR. Despite not being triggered as an error, I would avoid those out-scope log information.

@AfonsoSantos96
Copy link
Member Author

While testing this inside the partitioner, I'm getting this log output:

== Checking McCabe Cyclomatic Complexity metric ==
At src/core/main.c(4): complex_function has a complexity of 12. The maximum is 10
"/home/daniel/workspace/osyxtech/bao-partitioner/src/arch/armv8/inc/sysregs.h", line 83: expected pretty much anything after closing paren of function, but got EOF

== Check done with 1 error(s) ==

== Checking the number of called functions ==
At /core/main.c (81): calling_function calls 8 functions.The maximum are 7 calls
cflow:/home/daniel/workspace/osyxtech/bao-partitioner/src/core/console.c:37: chars_writen redefined
cflow:/home/daniel/workspace/osyxtech/bao-partitioner/src/core/console.c:33: this is the place of previous definition
== Check done with 1 error(s) ==

make: *** [ci/ci.mk:299: his-check] Error 255

The first report seems correct, since I introduce those +7 function calls. The next two reports regarding console.c seem that the tool is checking other properties outside the scope of the his metric introduced in this PR. Despite not being triggered as an error, I would avoid those out-scope log information.

Nicely spotted! During my tests that stderr didn't appear to me. Now that I re-ran your setup, I can see the error message.
To fix this, I redirected all stderr messages of all tools to /dev/null

@AfonsoSantos96 AfonsoSantos96 force-pushed the wip/his_complexity_metric branch 2 times, most recently from ffdbe14 to 1d6bdbe Compare October 27, 2023 11:37
@josecm josecm changed the base branch from wip/his_complexity_metric to wip/his-metrics October 27, 2023 13:06
@AfonsoSantos96 AfonsoSantos96 force-pushed the wip/his_calls branch 3 times, most recently from 6a4e321 to 9d71df0 Compare November 9, 2023 11:56
Copy link
Member

@danielRep danielRep left a comment

Choose a reason for hiding this comment

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

Small change. Tested it and LGTM.

his_checker.py Outdated Show resolved Hide resolved
@josecm josecm self-assigned this Feb 16, 2024
@danielRep danielRep force-pushed the wip/his-metrics branch 5 times, most recently from c1452fe to 4a2edbb Compare February 27, 2024 17:00
@danielRep danielRep changed the title feat(HIS): add function calls metric check to script feat(HIS): add CALLS his metric Feb 29, 2024
@danielRep danielRep marked this pull request as draft February 29, 2024 13:43
@danielRep
Copy link
Member

I had the change substantially how this metric has been done. Previously the cflow was cap to a depth of 2 which lead to some calls not being capture. To verify this just run cflow -l --depth=2 src/calls/calls.c vs cflow -l src/calls/calls.cusing the bao-his-tests.
With the newer changes we construct a tree from cflow output to handle data. Each node represents a function call and through a traversal process we identify functions with a greater number of functions calls than the permitted limit/threshold.

@danielRep danielRep marked this pull request as ready for review March 8, 2024 13:05
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.

3 participants