You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by DaanHoogland <gi...@git.apache.org> on 2015/06/17 11:07:32 UTC
[GitHub] cloudstack pull request: findbugs: deal with all the encoding issu...
GitHub user DaanHoogland opened a pull request:
https://github.com/apache/cloudstack/pull/467
findbugs: deal with all the encoding issues in a unified way
further getBytes() calls can
getBytes(StringUtils.getPrefferedCharset()) instead
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/DaanHoogland/cloudstack master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/467.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 #467
----
commit 84faad96b62e523527415e4b1b9a261200194059
Author: Daan Hoogland <da...@gmail.com>
Date: 2015-06-17T08:47:03Z
findbugs: deal with all the encoding issues in a unified way
further getBytes() calls can
getBytes(StringUtils.getPrefferedCharset()) instead
----
---
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: findbugs: deal with all the encoding issu...
Posted by rsafonseca <gi...@git.apache.org>.
Github user rsafonseca commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/467#discussion_r32617440
--- Diff: utils/src/com/cloud/utils/StringUtils.java ---
@@ -32,6 +33,21 @@
public class StringUtils {
private static final char[] hexChar = {'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D', 'E', 'F'};
+ private static Charset _ACS_Charset;
+
+ {
+ String preferredCharset = "UTF8";
--- End diff --
Actually it would work, UTF8 is a supported alias for the UTF-8 charset. I do prefer to always use the actual register charset name "UTF-8" though ;)
---
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: findbugs: deal with all the encoding issu...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/467#issuecomment-112795850
I seem to have messed up here and put two unrelated commits in one pr. @karuturi, you want me to clean up my mess or can we merge it like this?
---
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: findbugs: deal with all the encoding issu...
Posted by rsafonseca <gi...@git.apache.org>.
Github user rsafonseca commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/467#discussion_r32617957
--- Diff: plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java ---
@@ -1094,13 +1092,6 @@ private String createOVAFromMetafile(String metafileName) throws Exception {
} catch (Exception e) {
s_logger.error("Exception while creating OVA using Metafile", e);
throw e;
- } finally {
- if (strm != null) {
- try {
- strm.close();
--- End diff --
Since you removed the finally block, the FileInputStream is not getting closed. Put it at the end of the try block at least :)
---
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: findbugs: deal with all the encoding issu...
Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/467#discussion_r32618313
--- Diff: plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java ---
@@ -1094,13 +1092,6 @@ private String createOVAFromMetafile(String metafileName) throws Exception {
} catch (Exception e) {
s_logger.error("Exception while creating OVA using Metafile", e);
throw e;
- } finally {
- if (strm != null) {
- try {
- strm.close();
--- End diff --
@rsafonseca since its a [try-with-resources Statement](http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html), it will get closed
---
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: findbugs: deal with all the encoding issu...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/467#issuecomment-120524298
yes this is on master. is there doubt?
---
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: findbugs: deal with all the encoding issu...
Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/467#issuecomment-120495209
@DaanHoogland was this committed to master?
---
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: findbugs: deal with all the encoding issu...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/467#discussion_r32616885
--- Diff: utils/src/com/cloud/utils/StringUtils.java ---
@@ -32,6 +33,21 @@
public class StringUtils {
private static final char[] hexChar = {'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D', 'E', 'F'};
+ private static Charset _ACS_Charset;
+
+ {
+ String preferredCharset = "UTF8";
--- End diff --
so are so absolutely right. My code would silently fall back to that as it is the default if file.encoding is not set. bad bug, will fix.
---
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: findbugs: deal with all the encoding issu...
Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on the pull request:
https://github.com/apache/cloudstack/pull/467#issuecomment-112767792
since its a [try-with-resources Statement](http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html), it will get closed
---
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: findbugs: deal with all the encoding issu...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/467
---
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: findbugs: deal with all the encoding issu...
Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/467#issuecomment-120541217
@DaanHoogland some weird confusion, nevermind
---
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: findbugs: deal with all the encoding issu...
Posted by rsafonseca <gi...@git.apache.org>.
Github user rsafonseca commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/467#discussion_r32618952
--- Diff: plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java ---
@@ -1094,13 +1092,6 @@ private String createOVAFromMetafile(String metafileName) throws Exception {
} catch (Exception e) {
s_logger.error("Exception while creating OVA using Metafile", e);
throw e;
- } finally {
- if (strm != null) {
- try {
- strm.close();
--- End diff --
Nice, i was unaware of that. Thx for the link Rajani, I'll start using
try-with-resources as well :D
On Jun 17, 2015 1:51 PM, "Rajani Karuturi" <no...@github.com> wrote:
> In
> plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java
> <https://github.com/apache/cloudstack/pull/467#discussion_r32618313>:
>
> > @@ -1094,13 +1092,6 @@ private String createOVAFromMetafile(String metafileName) throws Exception {
> > } catch (Exception e) {
> > s_logger.error("Exception while creating OVA using Metafile", e);
> > throw e;
> > - } finally {
> > - if (strm != null) {
> > - try {
> > - strm.close();
>
> @rsafonseca <https://github.com/rsafonseca> since its a try-with-resources
> Statement
> <http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html>,
> it will get closed
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/cloudstack/pull/467/files#r32618313>.
>
---
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: findbugs: deal with all the encoding issu...
Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/467#discussion_r32616592
--- Diff: utils/src/com/cloud/utils/StringUtils.java ---
@@ -32,6 +33,21 @@
public class StringUtils {
private static final char[] hexChar = {'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D', 'E', 'F'};
+ private static Charset _ACS_Charset;
+
+ {
+ String preferredCharset = "UTF8";
--- End diff --
Isn't the charset name UTF-8?
---
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.
---
Re: [GitHub] cloudstack pull request: findbugs: deal with all the
encoding issu...
Posted by Daan Hoogland <da...@gmail.com>.
yeah, I hate that naming convention but I'll remove the _ from the name.
On Thu, Jun 18, 2015 at 5:50 AM, karuturi <gi...@git.apache.org> wrote:
> Github user karuturi commented on the pull request:
>
> https://github.com/apache/cloudstack/pull/467#issuecomment-113027041
>
> @DaanHoogland looks like you split the commit :) (the combined one was also ok. )
>
> I see some checkstyle errors in the jenkins build
> [INFO] --- maven-checkstyle-plugin:2.11:check (cloudstack-checkstyle) @ cloud-utils ---
> [INFO] Starting audit...
> /home/jenkins/jenkins-slave/workspace/cloudstack-pull-requests/utils/src/com/cloud/utils/StringUtils.java:36:28: Name '_acsCharset' must match pattern '^(s_)?[a-z][a-zA-Z0-9]*$'.
> Audit done.
>
>
>
> ---
> 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.
> ---
--
Daan
[GitHub] cloudstack pull request: findbugs: deal with all the encoding issu...
Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on the pull request:
https://github.com/apache/cloudstack/pull/467#issuecomment-113027041
@DaanHoogland looks like you split the commit :) (the combined one was also ok. )
I see some checkstyle errors in the jenkins build
[INFO] --- maven-checkstyle-plugin:2.11:check (cloudstack-checkstyle) @ cloud-utils ---
[INFO] Starting audit...
/home/jenkins/jenkins-slave/workspace/cloudstack-pull-requests/utils/src/com/cloud/utils/StringUtils.java:36:28: Name '_acsCharset' must match pattern '^(s_)?[a-z][a-zA-Z0-9]*$'.
Audit done.
---
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.
---