You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by kansal <gi...@git.apache.org> on 2015/07/30 12:55:35 UTC
[GitHub] cloudstack pull request: Coverity Issue: Resource Leak fixed
GitHub user kansal opened a pull request:
https://github.com/apache/cloudstack/pull/643
Coverity Issue: Resource Leak fixed
Resource leak fixed. Please review.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kansal/cloudstack Coverity-17897
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/643.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #643
----
commit 8f083a5e4087c0a89f16e16ccfad592d678cbc09
Author: Kshitij Kansal <ks...@citrix.com>
Date: 2015-07-30T10:53:50Z
Coverity Issue: Resource Leak fixed
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Coverity Issue: Resource Leak fixed
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/643
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Coverity Issue: Resource Leak fixed
Posted by miguelaferreira <gi...@git.apache.org>.
Github user miguelaferreira commented on the pull request:
https://github.com/apache/cloudstack/pull/643#issuecomment-126583069
Cool :+1:
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Coverity Issue: Resource Leak fixed
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/643#issuecomment-126599320
Thanks @kansal , please change the commit message to point to it. For instance:
CLOUDSTACK-8692: resource leak found by the internal coverity instance at <company or department or persons kitchen table>
Otherwise I'm all right with it
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Coverity Issue: Resource Leak fixed
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/643#discussion_r35860179
--- Diff: server/src/org/apache/cloudstack/region/RegionsApiUtil.java ---
@@ -186,8 +186,12 @@ protected static UserAccount makeUserAccountAPICall(Region region, String comman
XStream xstream = new XStream(new DomDriver());
xstream.alias("useraccount", UserAccountVO.class);
xstream.aliasField("id", UserAccountVO.class, "uuid");
- ObjectInputStream in = xstream.createObjectInputStream(is);
- return (UserAccountVO)in.readObject();
+ try(ObjectInputStream in = xstream.createObjectInputStream(is);) {
+ return (UserAccountVO)in.readObject();
+ } catch (IOException e) {
+ s_logger.error(e.getMessage());
+ return null;
--- End diff --
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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Coverity Issue: Resource Leak fixed
Posted by miguelaferreira <gi...@git.apache.org>.
Github user miguelaferreira commented on the pull request:
https://github.com/apache/cloudstack/pull/643#issuecomment-126319900
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 form those internal systems it would be extremely hard to backtrack. Don't you think?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Coverity Issue: Resource Leak fixed
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/643#issuecomment-127169469
thanks @kansal LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Coverity Issue: Resource Leak fixed
Posted by kansal <gi...@git.apache.org>.
Github user kansal commented on the pull request:
https://github.com/apache/cloudstack/pull/643#issuecomment-126570824
@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 :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Coverity Issue: Resource Leak fixed
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/643#issuecomment-126319443
-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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Coverity Issue: Resource Leak fixed
Posted by kansal <gi...@git.apache.org>.
Github user kansal commented on the pull request:
https://github.com/apache/cloudstack/pull/643#issuecomment-126301540
@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."
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Coverity Issue: Resource Leak fixed
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/643#issuecomment-126274712
@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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---