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

Fix the set_sec_mod handler #1276

Closed
wants to merge 1 commit into from

Conversation

nichamon
Copy link
Collaborator

Without the patch, the handler does not send the error code back to the interface when the given UID or GID is invalid.

Without the patch, the handler does not send the error code back
to the interface when the given UID or GID is invalid.
@nichamon nichamon requested a review from tom95858 August 30, 2023 17:28
@nichamon
Copy link
Collaborator Author

The bugs were found by running LDMSD against the set_sec_mod_test in the ldms-test. The set_sec_mod_test isn't a part of the routine test without this PR.

@@ -5539,6 +5539,7 @@ static int set_sec_mod_handler(ldmsd_req_ctxt_t reqc)
if (isdigit(value[0])) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@nichamon the value variable here is leaked throughout this function. Although it is freed at the end, because it is reused throughout, only the last value is actually freed. This fix is part of the "memory-leak" branch I was going to send you, but I think my change and yours will conflict. Maybe I just send you the branch and you combine them all into one pull request that you submit?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll do that.

@tom95858
Copy link
Collaborator

tom95858 commented Sep 2, 2023

@nichamon is this patch subsumed by the other one? Can we just close it?

@nichamon
Copy link
Collaborator Author

nichamon commented Sep 2, 2023

Yes, it is a part of #1278. I'm closing this.

@nichamon nichamon closed this Sep 2, 2023
@nichamon nichamon deleted the fix-set_sec_mod branch October 16, 2023 14:29
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.

2 participants