Skip to content

out_stackdriver: fixed a memory leak (CID 508244) - #9299

Merged
edsiper merged 2 commits into
masterfrom
leonardo-master-coverity-issue-508244
Aug 29, 2024
Merged

out_stackdriver: fixed a memory leak (CID 508244)#9299
edsiper merged 2 commits into
masterfrom
leonardo-master-coverity-issue-508244

Conversation

@leonardo-albertovich

Copy link
Copy Markdown
Contributor

No description provided.

Signed-off-by: Leonardo Alminana <leonardo.alminana@chronosphere.io>
@leonardo-albertovich

Copy link
Copy Markdown
Contributor Author

@igorpeshansky I performed a cursory inspection to validate (and expand) the findings and patched the leaks I could confirm, however I'm not familiar with the stackdriver plugin and would greatly appreciate if someone from the team could verify this PR.

Additionally, It seems that source_location_file and source_location_function are also leaked, could you verify this?

@braydonk

braydonk commented Aug 29, 2024

Copy link
Copy Markdown
Contributor

Thanks for fixing this. These cleanups were missed when I reviewed the PRs by external contributors, I should have caught them.

source_location_file and source_location_function don't look like they should be leaked, at least a look at the code indicates they should be cleaned up.

They are allocated here (not guarded by any condition):

source_location_file = flb_sds_create("");
source_location_line = 0;
source_location_function = flb_sds_create("");
source_location_extra_size = 0;

Then cleaned up here (also not guarded by any conditions):

flb_sds_destroy(source_location_file);
flb_sds_destroy(source_location_function);

@leonardo-albertovich

Copy link
Copy Markdown
Contributor Author

I meant they would be leaked if this code path was taken.

@braydonk

Copy link
Copy Markdown
Contributor

I didn't notice that, yeah you're right that's a leak.

Signed-off-by: Leonardo Alminana <leonardo.alminana@chronosphere.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants