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

Refactor "Custom" code (not really customizable) #70

Closed
2 tasks done
jphickey opened this issue Feb 22, 2023 · 1 comment · Fixed by #90
Closed
2 tasks done

Refactor "Custom" code (not really customizable) #70

jphickey opened this issue Feb 22, 2023 · 1 comment · Fixed by #90

Comments

@jphickey
Copy link
Contributor

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
The structure of the HS code includes hs_custom.h and hs_custom.c that imply that these contain user-defined functions. However the current code does not really allow for this. Notably -

  1. The interface between "custom" and standard routines is not well defined
  2. Unit tests directly use the hs_custom_internal.h data structures in non-custom tests (e.g. see HS_MonitorUtilization_Test_CPUHogging, among others)

Furthermore, the code is simply in the same src directory as the rest of HS, so any customizations mean creating a fork of HS and the user having to manage that fork.

To Reproduce
Attempt to change the contents of HS_CustomData_t - such as removing the UtilMult and and UtilDiv fields and replacing with some other logic. This will break seeming unrelated code that assumes these fields exist.

Expected behavior
If HS depends on a computation working a certain way, it shouldn't be labeled as custom. Conversely, if a section of code is truly intended to be customized by the user, the interface into that custom function needs to be well-defined and no other parts of the code should assume it works a certain way or has certain fields in its global data.

Code snips
Example of a place where unit test is directly accessing custom fields from a non-custom test:

HS_CustomData.LastIdleTaskInterval = 1;
HS_CustomData.UtilMult1 = -3;
HS_CustomData.UtilMult2 = 1;
HS_CustomData.UtilDiv = 1;

System observed on:
N/A

Additional context
Could this custom logic potentially be refactored into a separate support library, so the user would not have to fork HS to customize these routines?

Reporter Info
Joseph Hickey, Vantage Systems, Inc.

@jphickey jphickey linked a pull request Apr 20, 2023 that will close this issue
2 tasks
@jphickey
Copy link
Contributor Author

This was fixed in PR #90, no longer using "custom" logic for this in HS - the functionality is moved to the PSP, so this is no longer relevant.

@dmknutsen dmknutsen added this to the Equuleus milestone May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants