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

[CBRD-25359] uhosts version of freeaddrinfo () to release memory explicitly #5202

Conversation

kisoo-han
Copy link
Contributor

http://jira.cubrid.org/browse/CBRD-25359

Description

  • because uhosts alloc memory using malloc () and free using freeaddrinfo (), CUBRID memory utility cannot catch memory release point inside freeaddrinfo (3) glibc library.
  • we will introduce new function, freeaddrinfo_uhost (), and in case uhosts enabled, we will free memory allocated by getaddrinfo () externally using free (3).

@kisoo-han kisoo-han self-assigned this May 15, 2024
@kisoo-han kisoo-han requested review from Rudeus and hornetmj May 15, 2024 23:46

if (res)
{
FREE_MEM (res->ai_canonname);
Copy link
Contributor

Choose a reason for hiding this comment

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

It is good to me to replace malloc to MALLOC() macro if you want to use FREE_MEM() macro. Or how about use free()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's a good idea to use MALLOC and FREE_MEM as a pair in point of memory managements.
fixed.

@kisoo-han kisoo-han requested a review from hornetmj May 17, 2024 01:48
@Rudeus
Copy link

Rudeus commented May 17, 2024

Since this issue occurs in a situation where memory monitoring project is applied, I believe it is appropriate to change the target branch for merging from the develop branch to the feature/memory_monitoring_ver2_merge branch.

@kisoo-han kisoo-han changed the base branch from develop to feature/memory_monitoring_ver2_merge May 17, 2024 09:15
@hornetmj
Copy link
Contributor

Since this issue occurs in a situation where memory monitoring project is applied, I believe it is appropriate to change the target branch for merging from the develop branch to the feature/memory_monitoring_ver2_merge branch.

@kisoo-han To explain further, we are performing regression testing in {{memory_monitoring_ver2_merge}} branch. We requested this because we thought it would be better if the changes made in this PR were verified together. I feel like the request was made without sufficient reason, so I would like to explain further. thank you for the quick response. Also, I apologize for the sudden change request.

@kisoo-han kisoo-han changed the base branch from feature/memory_monitoring_ver2_merge to develop May 17, 2024 09:20
@kisoo-han kisoo-han closed this May 17, 2024
@kisoo-han
Copy link
Contributor Author

This issue will be moved to #5211 due to base branch problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants