You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Ritu Sabharwal <rs...@brocade.com> on 2014/07/30 21:44:11 UTC
Review Request 24111: Coverity findings for brocade-plugin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24111/
-----------------------------------------------------------
Review request for cloudstack and Hugo Trippaers.
Bugs: CLOUDSTACK-6823
https://issues.apache.org/jira/browse/CLOUDSTACK-6823
Repository: cloudstack-git
Description
-------
Fixed Coverity findings for brocade-plugin
Diffs
-----
plugins/network-elements/brocade-vcs/src/com/cloud/api/response/BrocadeVcsDeviceResponse.java 60edbcf
plugins/network-elements/brocade-vcs/src/com/cloud/network/brocade/BrocadeVcsApi.java d5f06f8
Diff: https://reviews.apache.org/r/24111/diff/
Testing
-------
Thanks,
Ritu Sabharwal
Re: Review Request 24111: Coverity findings for brocade-plugin
Posted by Ritu Sabharwal <rs...@brocade.com>.
> On July 31, 2014, 4:19 a.m., Santhosh Edukulla wrote:
> > plugins/network-elements/brocade-vcs/src/com/cloud/network/brocade/BrocadeVcsApi.java, line 482
> > <https://reviews.apache.org/r/24111/diff/1/?file=646144#file646144line482>
> >
> > Message info seems to be corrected.
Hi Santhosh,
What is fix required here?
Thanks & Regards,
Ritu.
> On July 31, 2014, 4:19 a.m., Santhosh Edukulla wrote:
> > plugins/network-elements/brocade-vcs/src/com/cloud/network/brocade/BrocadeVcsApi.java, line 561
> > <https://reviews.apache.org/r/24111/diff/1/?file=646144#file646144line561>
> >
> > Missing try\catch here. But, there is a better way suggested by Daan to auto close resources using try-with-resource to avoid any of these usages.
I will fix this using try-with-resources.
- Ritu
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24111/#review49200
-----------------------------------------------------------
On July 30, 2014, 7:44 p.m., Ritu Sabharwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24111/
> -----------------------------------------------------------
>
> (Updated July 30, 2014, 7:44 p.m.)
>
>
> Review request for cloudstack and Hugo Trippaers.
>
>
> Bugs: CLOUDSTACK-6823
> https://issues.apache.org/jira/browse/CLOUDSTACK-6823
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Fixed Coverity findings for brocade-plugin
>
>
> Diffs
> -----
>
> plugins/network-elements/brocade-vcs/src/com/cloud/api/response/BrocadeVcsDeviceResponse.java 60edbcf
> plugins/network-elements/brocade-vcs/src/com/cloud/network/brocade/BrocadeVcsApi.java d5f06f8
>
> Diff: https://reviews.apache.org/r/24111/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ritu Sabharwal
>
>
Re: Review Request 24111: Coverity findings for brocade-plugin
Posted by Santhosh Edukulla <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24111/#review49200
-----------------------------------------------------------
plugins/network-elements/brocade-vcs/src/com/cloud/network/brocade/BrocadeVcsApi.java
<https://reviews.apache.org/r/24111/#comment86076>
Message info seems to be corrected.
plugins/network-elements/brocade-vcs/src/com/cloud/network/brocade/BrocadeVcsApi.java
<https://reviews.apache.org/r/24111/#comment86079>
Missing try\catch here. But, there is a better way suggested by Daan to auto close resources using try-with-resource to avoid any of these usages.
- Santhosh Edukulla
On July 30, 2014, 7:44 p.m., Ritu Sabharwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24111/
> -----------------------------------------------------------
>
> (Updated July 30, 2014, 7:44 p.m.)
>
>
> Review request for cloudstack and Hugo Trippaers.
>
>
> Bugs: CLOUDSTACK-6823
> https://issues.apache.org/jira/browse/CLOUDSTACK-6823
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Fixed Coverity findings for brocade-plugin
>
>
> Diffs
> -----
>
> plugins/network-elements/brocade-vcs/src/com/cloud/api/response/BrocadeVcsDeviceResponse.java 60edbcf
> plugins/network-elements/brocade-vcs/src/com/cloud/network/brocade/BrocadeVcsApi.java d5f06f8
>
> Diff: https://reviews.apache.org/r/24111/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ritu Sabharwal
>
>
Re: Review Request 24111: Coverity findings for brocade-plugin
Posted by Santhosh Edukulla <sa...@citrix.com>.
> On Aug. 5, 2014, 9:31 a.m., Santhosh Edukulla wrote:
> > Ship It!
Pushed to master
Updated Branches:
refs/heads/master 1bfb1f650 -> cc725e53e
- Santhosh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24111/#review49578
-----------------------------------------------------------
On July 31, 2014, 5:57 p.m., Ritu Sabharwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24111/
> -----------------------------------------------------------
>
> (Updated July 31, 2014, 5:57 p.m.)
>
>
> Review request for cloudstack and Hugo Trippaers.
>
>
> Bugs: CLOUDSTACK-6823
> https://issues.apache.org/jira/browse/CLOUDSTACK-6823
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Fixed Coverity findings for brocade-plugin
>
>
> Diffs
> -----
>
> plugins/network-elements/brocade-vcs/src/com/cloud/api/response/BrocadeVcsDeviceResponse.java 60edbcf
> plugins/network-elements/brocade-vcs/src/com/cloud/network/brocade/BrocadeVcsApi.java d5f06f8
>
> Diff: https://reviews.apache.org/r/24111/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ritu Sabharwal
>
>
Re: Review Request 24111: Coverity findings for brocade-plugin
Posted by Santhosh Edukulla <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24111/#review49578
-----------------------------------------------------------
Ship it!
Ship It!
- Santhosh Edukulla
On July 31, 2014, 5:57 p.m., Ritu Sabharwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24111/
> -----------------------------------------------------------
>
> (Updated July 31, 2014, 5:57 p.m.)
>
>
> Review request for cloudstack and Hugo Trippaers.
>
>
> Bugs: CLOUDSTACK-6823
> https://issues.apache.org/jira/browse/CLOUDSTACK-6823
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Fixed Coverity findings for brocade-plugin
>
>
> Diffs
> -----
>
> plugins/network-elements/brocade-vcs/src/com/cloud/api/response/BrocadeVcsDeviceResponse.java 60edbcf
> plugins/network-elements/brocade-vcs/src/com/cloud/network/brocade/BrocadeVcsApi.java d5f06f8
>
> Diff: https://reviews.apache.org/r/24111/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ritu Sabharwal
>
>
Re: Review Request 24111: Coverity findings for brocade-plugin
Posted by Santhosh Edukulla <sa...@citrix.com>.
> On Aug. 1, 2014, 5:57 a.m., Santhosh Edukulla wrote:
> > plugins/network-elements/brocade-vcs/src/com/cloud/network/brocade/BrocadeVcsApi.java, line 547
> > <https://reviews.apache.org/r/24111/diff/2/?file=646948#file646948line547>
> >
> > It seems there is no corresponding catch here, did compilation worked?
>
> Ritu Sabharwal wrote:
> Hi Santhosh,
>
> This log statement is not there in the latest patch that I submitted.
>
> There is a throws clause in the method signature private String responseToErrorMessage(HttpResponse response) throws IOException and the IOException is handled in the caller methods.
>
> The compilation works fine.
>
> Thanks,
> Ritu S.
It seems that patch was not in correct git format, we can do "git apply" and "git commit -m --author=...", if its ok.
Hugo,
if we don't have any other concerns, we can push i believe.
Santhosh
- Santhosh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24111/#review49323
-----------------------------------------------------------
On July 31, 2014, 5:57 p.m., Ritu Sabharwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24111/
> -----------------------------------------------------------
>
> (Updated July 31, 2014, 5:57 p.m.)
>
>
> Review request for cloudstack and Hugo Trippaers.
>
>
> Bugs: CLOUDSTACK-6823
> https://issues.apache.org/jira/browse/CLOUDSTACK-6823
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Fixed Coverity findings for brocade-plugin
>
>
> Diffs
> -----
>
> plugins/network-elements/brocade-vcs/src/com/cloud/api/response/BrocadeVcsDeviceResponse.java 60edbcf
> plugins/network-elements/brocade-vcs/src/com/cloud/network/brocade/BrocadeVcsApi.java d5f06f8
>
> Diff: https://reviews.apache.org/r/24111/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ritu Sabharwal
>
>
Re: Review Request 24111: Coverity findings for brocade-plugin
Posted by Ritu Sabharwal <rs...@brocade.com>.
> On Aug. 1, 2014, 5:57 a.m., Santhosh Edukulla wrote:
> > plugins/network-elements/brocade-vcs/src/com/cloud/network/brocade/BrocadeVcsApi.java, line 547
> > <https://reviews.apache.org/r/24111/diff/2/?file=646948#file646948line547>
> >
> > It seems there is no corresponding catch here, did compilation worked?
Hi Santhosh,
This log statement is not there in the latest patch that I submitted.
There is a throws clause in the method signature private String responseToErrorMessage(HttpResponse response) throws IOException and the IOException is handled in the caller methods.
The compilation works fine.
Thanks,
Ritu S.
- Ritu
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24111/#review49323
-----------------------------------------------------------
On July 31, 2014, 5:57 p.m., Ritu Sabharwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24111/
> -----------------------------------------------------------
>
> (Updated July 31, 2014, 5:57 p.m.)
>
>
> Review request for cloudstack and Hugo Trippaers.
>
>
> Bugs: CLOUDSTACK-6823
> https://issues.apache.org/jira/browse/CLOUDSTACK-6823
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Fixed Coverity findings for brocade-plugin
>
>
> Diffs
> -----
>
> plugins/network-elements/brocade-vcs/src/com/cloud/api/response/BrocadeVcsDeviceResponse.java 60edbcf
> plugins/network-elements/brocade-vcs/src/com/cloud/network/brocade/BrocadeVcsApi.java d5f06f8
>
> Diff: https://reviews.apache.org/r/24111/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ritu Sabharwal
>
>
Re: Review Request 24111: Coverity findings for brocade-plugin
Posted by Santhosh Edukulla <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24111/#review49323
-----------------------------------------------------------
plugins/network-elements/brocade-vcs/src/com/cloud/network/brocade/BrocadeVcsApi.java
<https://reviews.apache.org/r/24111/#comment86298>
It seems there is no corresponding catch here, did compilation worked?
- Santhosh Edukulla
On July 31, 2014, 5:57 p.m., Ritu Sabharwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24111/
> -----------------------------------------------------------
>
> (Updated July 31, 2014, 5:57 p.m.)
>
>
> Review request for cloudstack and Hugo Trippaers.
>
>
> Bugs: CLOUDSTACK-6823
> https://issues.apache.org/jira/browse/CLOUDSTACK-6823
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Fixed Coverity findings for brocade-plugin
>
>
> Diffs
> -----
>
> plugins/network-elements/brocade-vcs/src/com/cloud/api/response/BrocadeVcsDeviceResponse.java 60edbcf
> plugins/network-elements/brocade-vcs/src/com/cloud/network/brocade/BrocadeVcsApi.java d5f06f8
>
> Diff: https://reviews.apache.org/r/24111/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ritu Sabharwal
>
>
Re: Review Request 24111: Coverity findings for brocade-plugin
Posted by Ritu Sabharwal <rs...@brocade.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24111/
-----------------------------------------------------------
(Updated July 31, 2014, 5:57 p.m.)
Review request for cloudstack and Hugo Trippaers.
Changes
-------
Fixed the open issues.
Bugs: CLOUDSTACK-6823
https://issues.apache.org/jira/browse/CLOUDSTACK-6823
Repository: cloudstack-git
Description
-------
Fixed Coverity findings for brocade-plugin
Diffs (updated)
-----
plugins/network-elements/brocade-vcs/src/com/cloud/api/response/BrocadeVcsDeviceResponse.java 60edbcf
plugins/network-elements/brocade-vcs/src/com/cloud/network/brocade/BrocadeVcsApi.java d5f06f8
Diff: https://reviews.apache.org/r/24111/diff/
Testing
-------
Thanks,
Ritu Sabharwal
Re: Review Request 24111: Coverity findings for brocade-plugin
Posted by Ritu Sabharwal <rs...@brocade.com>.
> On July 31, 2014, 7:01 a.m., Santhosh Edukulla wrote:
> > plugins/network-elements/brocade-vcs/src/com/cloud/network/brocade/BrocadeVcsApi.java, line 482
> > <https://reviews.apache.org/r/24111/diff/1/?file=646144#file646144line482>
> >
> > This particular try\except is for an IO exception possible when closing a resource, so the message should have some reflection in it. But, when you use try-with-resource as mentioned in other issue, this is not required.
Fixed this also.
- Ritu
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24111/#review49213
-----------------------------------------------------------
On July 31, 2014, 5:57 p.m., Ritu Sabharwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24111/
> -----------------------------------------------------------
>
> (Updated July 31, 2014, 5:57 p.m.)
>
>
> Review request for cloudstack and Hugo Trippaers.
>
>
> Bugs: CLOUDSTACK-6823
> https://issues.apache.org/jira/browse/CLOUDSTACK-6823
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Fixed Coverity findings for brocade-plugin
>
>
> Diffs
> -----
>
> plugins/network-elements/brocade-vcs/src/com/cloud/api/response/BrocadeVcsDeviceResponse.java 60edbcf
> plugins/network-elements/brocade-vcs/src/com/cloud/network/brocade/BrocadeVcsApi.java d5f06f8
>
> Diff: https://reviews.apache.org/r/24111/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ritu Sabharwal
>
>
Re: Review Request 24111: Coverity findings for brocade-plugin
Posted by Santhosh Edukulla <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24111/#review49213
-----------------------------------------------------------
plugins/network-elements/brocade-vcs/src/com/cloud/network/brocade/BrocadeVcsApi.java
<https://reviews.apache.org/r/24111/#comment86092>
This particular try\except is for an IO exception possible when closing a resource, so the message should have some reflection in it. But, when you use try-with-resource as mentioned in other issue, this is not required.
- Santhosh Edukulla
On July 30, 2014, 7:44 p.m., Ritu Sabharwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24111/
> -----------------------------------------------------------
>
> (Updated July 30, 2014, 7:44 p.m.)
>
>
> Review request for cloudstack and Hugo Trippaers.
>
>
> Bugs: CLOUDSTACK-6823
> https://issues.apache.org/jira/browse/CLOUDSTACK-6823
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Fixed Coverity findings for brocade-plugin
>
>
> Diffs
> -----
>
> plugins/network-elements/brocade-vcs/src/com/cloud/api/response/BrocadeVcsDeviceResponse.java 60edbcf
> plugins/network-elements/brocade-vcs/src/com/cloud/network/brocade/BrocadeVcsApi.java d5f06f8
>
> Diff: https://reviews.apache.org/r/24111/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ritu Sabharwal
>
>