Skip to content

Coverity Issue: Resource Leak fixed#643

Merged
asfgit merged 1 commit into
apache:masterfrom
kansal:Coverity-17897
Aug 3, 2015
Merged

Coverity Issue: Resource Leak fixed#643
asfgit merged 1 commit into
apache:masterfrom
kansal:Coverity-17897

Conversation

@kansal
Copy link
Copy Markdown
Contributor

@kansal kansal commented Jul 30, 2015

Resource leak fixed. Please review.

@asfbot
Copy link
Copy Markdown

asfbot commented Jul 30, 2015

cloudstack-pull-rats #159 SUCCESS
This pull request looks good

@DaanHoogland
Copy link
Copy Markdown
Contributor

@kansal please add the coverity number to the commit message, this way we can not track it. And while you look at it make sure you put the issue on your name to prevent any double effort. I think we actually voted for not accepting any commits without ref anymore.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this return is not needed. we will fall through to other return. You can opt not to and put another log message in there as well. (no biggy)

@asfbot
Copy link
Copy Markdown

asfbot commented Jul 30, 2015

cloudstack-pull-requests #857 SUCCESS
This pull request looks good

@kansal
Copy link
Copy Markdown
Contributor Author

kansal commented Jul 30, 2015

@DaanHoogland It was the issue that was reported by our internal coverity. The issue reported is:

"noescape: Resource 'in' is not closed or saved in readObject.
CID 17897: Resource leak (RESOURCE_LEAK). leaked_resource: Variable in going out of scope leaks the resource it refers to."

@asfbot
Copy link
Copy Markdown

asfbot commented Jul 30, 2015

cloudstack-pull-analysis #92 UNSTABLE
Looks like there's a problem with this pull request

@DaanHoogland
Copy link
Copy Markdown
Contributor

-1 on this. I cannot track any internal tracking system of another company. please if the public coverity doesn't have this make a jira ticket describve the problem and put the ticket id in hte commit message.

@miguelaferreira
Copy link
Copy Markdown
Contributor

Hi @kansal

If this issue was caught by your internal Coverity scan, wouldn't it also have been cough by the public Coverity scan? Or are these configured differently?

It would be great to use the public IDs all-around, otherwise it will be very confusing. I can imagine many people have their own internal ticketing systems (for example), and if people would just start to use IDs from those internal systems it would be extremely hard to backtrack. Don't you think?

@kansal
Copy link
Copy Markdown
Contributor Author

kansal commented Jul 31, 2015

@DaanHoogland @miguelaferreira I understand you concerns. I have created the ticket for this in JIRA with ID: CLOUDSTACK-8692. Will keep this in mind while for future commits. Thanks :)

@miguelaferreira
Copy link
Copy Markdown
Contributor

Cool 👍

@DaanHoogland
Copy link
Copy Markdown
Contributor

Thanks @kansal , please change the commit message to point to it. For instance:
CLOUDSTACK-8692: resource leak found by the internal coverity instance at

Otherwise I'm all right with it

@asfbot
Copy link
Copy Markdown

asfbot commented Aug 3, 2015

cloudstack-pull-rats #180 ABORTED

@asfbot
Copy link
Copy Markdown

asfbot commented Aug 3, 2015

cloudstack-pull-requests #878 ABORTED

@asfbot
Copy link
Copy Markdown

asfbot commented Aug 3, 2015

cloudstack-pull-analysis #113 SUCCESS
This pull request looks good

@DaanHoogland
Copy link
Copy Markdown
Contributor

thanks @kansal LGTM (merging)

@asfgit asfgit merged commit e0ba530 into apache:master Aug 3, 2015
asfgit pushed a commit that referenced this pull request Aug 3, 2015
Coverity Issue: Resource Leak fixed

* pr/643:
  CLOUDSTACK-8692: Resource leak found by the internal coverity instance at Citrix fixed.

Signed-off-by: Daan Hoogland <daan.hoogland@gmail.com>
yadvr pushed a commit that referenced this pull request Jan 20, 2021
Fixes #643

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
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.

5 participants