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

TS-4794 fix the memory leaks #855

Closed
wants to merge 6 commits into from

Conversation

bryonglodencissp
Copy link

On line no. 74 of 'test_P_DNS.cc' there is a memory leak and on line no. 72 of 'test_P_Net.cc' there is memory leak -- both memory leaks are errors.

Found by https://github.com/bryongloden/cppcheck

Bryon Gloden, CISSP® added 2 commits August 11, 2016 18:57
[iocore/dns/test_P_DNS.cc:74]: (error) Memory leak: str
[iocore/net/test_P_Net.cc:72]: (error) Memory leak: str
@zwoop zwoop added the DNS label Aug 12, 2016
@zwoop zwoop added this to the 7.0.0 milestone Aug 12, 2016
@zwoop zwoop self-assigned this Aug 12, 2016
@zwoop
Copy link
Contributor

zwoop commented Aug 12, 2016

[approve ci]

@atsci
Copy link

atsci commented Aug 12, 2016

Linux build failed! See https://ci.trafficserver.apache.org/job/Github-Linux/421/ for details.

@zwoop
Copy link
Contributor

zwoop commented Aug 12, 2016

Needs to run "make clang-format". :)

@atsci
Copy link

atsci commented Aug 12, 2016

FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/524/ for details.

Bryon Gloden, CISSP® added 2 commits August 15, 2016 15:14
[iocore/dns/test_P_DNS.cc:74]: (error) Memory leak: str
[iocore/net/test_P_Net.cc:72]: (error) Memory leak: str
@zwoop
Copy link
Contributor

zwoop commented Aug 16, 2016

Also would have a Jira # in the subject line please.

@bryonglodencissp
Copy link
Author

Greetings @zwoop 😬

Sorry, what is a JIRA number? I've never heard this before. And I know how to change the commit message; is this the same as the subject line? If so, what should I update it to say? 👍

@zwoop
Copy link
Contributor

zwoop commented Aug 17, 2016

Ah, so you have to create a Jira ticket (for now at least) in conjunction with the Github PR. Go to https://issues.apache.org/jira/browse/TS/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel and create an account (unless you already did), create a new Issue for the Traffic Server project, and it will give you a TS- number. That's the Jira issue number, and you should refer to it in all commits etc. for this particular issue.

@bryonglodencissp bryonglodencissp changed the title fix the memory leaks TS-4794 fix the memory leaks Aug 28, 2016
@bryonglodencissp
Copy link
Author

Thank you @zwoop 👍

REF: https://issues.apache.org/jira/browse/TS-4794

@@ -68,6 +68,7 @@ struct NetTesterSM : public Continuation {
default:
ink_release_assert(!"unknown event");
}
delete[] str;
Copy link
Contributor

Choose a reason for hiding this comment

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

str needs to be initialized to NULL.

@zwoop
Copy link
Contributor

zwoop commented Sep 3, 2016

@bryongloden please address James' concern, and we can land this (after we run it on the CI again).

Bryon Gloden, CISSP® added 2 commits September 3, 2016 15:17
initialize str to NULL
credit @jpeach
[1/2]
initialize str to NULL
credit @jpeach
[2/2]
@asfgit asfgit closed this in 500c7ba Sep 3, 2016
asfgit pushed a commit that referenced this pull request Sep 3, 2016
ericcarlschwartz pushed a commit to ericcarlschwartz/trafficserver that referenced this pull request Sep 29, 2016
ericcarlschwartz pushed a commit to ericcarlschwartz/trafficserver that referenced this pull request Sep 29, 2016
@zwoop zwoop modified the milestone: 7.0.0 May 4, 2017
bneradt pushed a commit to bneradt/trafficserver that referenced this pull request Feb 6, 2024
* Add an HTTP/2 related rate limiting

(cherry picked from commit 2706da4)

Conflicts:
proxy/http2/HTTP2.cc

* Updates to work with Yahoo version of 9.1.x

Co-authored-by: Masakazu Kitajo <maskit@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants