You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by hanm <gi...@git.apache.org> on 2016/11/07 22:12:00 UTC
[GitHub] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.
GitHub user hanm opened a pull request:
https://github.com/apache/zookeeper/pull/102
ZOOKEEPER-2628: Fix findbug warnings.
This PR fixed 19 find bug warnings except this one, which might require interface change that potentially could break client side apps.
Malicious code vulnerability Warnings
org.apache.zookeeper.ZooDefs$Ids.OPEN_ACL_UNSAFE is a mutable collection
Bug type MS_MUTABLE_COLLECTION (click for details)
In class org.apache.zookeeper.ZooDefs$Ids
Field org.apache.zookeeper.ZooDefs$Ids.OPEN_ACL_UNSAFE
At ZooDefs.java:[line 116]
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/hanm/zookeeper ZOOKEEPER-2628
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zookeeper/pull/102.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 #102
----
commit d6e059ef11dea0de725b7bebd14e7bba626bdf53
Author: Michael Han <ha...@cloudera.com>
Date: 2016-11-07T22:09:10Z
ZOOKEEPER-2628: Fix findbug warnings.
----
---
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] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/102#discussion_r89356303
--- Diff: src/java/main/org/apache/zookeeper/cli/DeleteCommand.java ---
@@ -56,14 +56,7 @@ public CliCommand parse(String[] cmdArgs) throws CliParseException {
}
private void retainCompatibility(String[] cmdArgs) throws CliParseException {
- // delete path [version]
if (args.length > 2) {
- // rewrite to option
- String [] newCmd = new String[4];
--- End diff --
These are dead code. newCmd was declared but not used so it's better to remove them.
---
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] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/102#discussion_r86885871
--- Diff: src/java/main/org/apache/jute/compiler/CGenerator.java ---
@@ -61,70 +61,88 @@ void genCode() throws IOException {
+ outputDirectory);
}
}
- FileWriter c = new FileWriter(new File(outputDirectory, mName+".c"));
- FileWriter h = new FileWriter(new File(outputDirectory, mName+".h"));
- h.write("/**\n");
- h.write("* Licensed to the Apache Software Foundation (ASF) under one\n");
- h.write("* or more contributor license agreements. See the NOTICE file\n");
- h.write("* distributed with this work for additional information\n");
- h.write("* regarding copyright ownership. The ASF licenses this file\n");
- h.write("* to you under the Apache License, Version 2.0 (the\n");
- h.write("* \"License\"); you may not use this file except in compliance\n");
- h.write("* with the License. You may obtain a copy of the License at\n");
- h.write("*\n");
- h.write("* http://www.apache.org/licenses/LICENSE-2.0\n");
- h.write("*\n");
- h.write("* Unless required by applicable law or agreed to in writing, software\n");
- h.write("* distributed under the License is distributed on an \"AS IS\" BASIS,\n");
- h.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n");
- h.write("* See the License for the specific language governing permissions and\n");
- h.write("* limitations under the License.\n");
- h.write("*/\n");
- h.write("\n");
+ FileWriter c = null, h = null;
+ try {
+ c = new FileWriter(new File(outputDirectory, mName + ".c"));
+ h = new FileWriter(new File(outputDirectory, mName + ".h"));
- c.write("/**\n");
- c.write("* Licensed to the Apache Software Foundation (ASF) under one\n");
- c.write("* or more contributor license agreements. See the NOTICE file\n");
- c.write("* distributed with this work for additional information\n");
- c.write("* regarding copyright ownership. The ASF licenses this file\n");
- c.write("* to you under the Apache License, Version 2.0 (the\n");
- c.write("* \"License\"); you may not use this file except in compliance\n");
- c.write("* with the License. You may obtain a copy of the License at\n");
- c.write("*\n");
- c.write("* http://www.apache.org/licenses/LICENSE-2.0\n");
- c.write("*\n");
- c.write("* Unless required by applicable law or agreed to in writing, software\n");
- c.write("* distributed under the License is distributed on an \"AS IS\" BASIS,\n");
- c.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n");
- c.write("* See the License for the specific language governing permissions and\n");
- c.write("* limitations under the License.\n");
- c.write("*/\n");
- c.write("\n");
+ h.write("/**\n");
+ h.write("* Licensed to the Apache Software Foundation (ASF) under one\n");
+ h.write("* or more contributor license agreements. See the NOTICE file\n");
+ h.write("* distributed with this work for additional information\n");
+ h.write("* regarding copyright ownership. The ASF licenses this file\n");
+ h.write("* to you under the Apache License, Version 2.0 (the\n");
+ h.write("* \"License\"); you may not use this file except in compliance\n");
+ h.write("* with the License. You may obtain a copy of the License at\n");
+ h.write("*\n");
+ h.write("* http://www.apache.org/licenses/LICENSE-2.0\n");
+ h.write("*\n");
+ h.write("* Unless required by applicable law or agreed to in writing, software\n");
+ h.write("* distributed under the License is distributed on an \"AS IS\" BASIS,\n");
+ h.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n");
+ h.write("* See the License for the specific language governing permissions and\n");
+ h.write("* limitations under the License.\n");
+ h.write("*/\n");
+ h.write("\n");
- h.write("#ifndef __"+mName.toUpperCase().replace('.','_')+"__\n");
- h.write("#define __"+mName.toUpperCase().replace('.','_')+"__\n");
+ c.write("/**\n");
+ c.write("* Licensed to the Apache Software Foundation (ASF) under one\n");
+ c.write("* or more contributor license agreements. See the NOTICE file\n");
+ c.write("* distributed with this work for additional information\n");
+ c.write("* regarding copyright ownership. The ASF licenses this file\n");
+ c.write("* to you under the Apache License, Version 2.0 (the\n");
+ c.write("* \"License\"); you may not use this file except in compliance\n");
+ c.write("* with the License. You may obtain a copy of the License at\n");
+ c.write("*\n");
+ c.write("* http://www.apache.org/licenses/LICENSE-2.0\n");
+ c.write("*\n");
+ c.write("* Unless required by applicable law or agreed to in writing, software\n");
+ c.write("* distributed under the License is distributed on an \"AS IS\" BASIS,\n");
+ c.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n");
+ c.write("* See the License for the specific language governing permissions and\n");
+ c.write("* limitations under the License.\n");
+ c.write("*/\n");
+ c.write("\n");
- h.write("#include \"recordio.h\"\n");
- for (Iterator<JFile> i = mInclFiles.iterator(); i.hasNext();) {
- JFile f = i.next();
- h.write("#include \""+f.getName()+".h\"\n");
- }
- // required for compilation from C++
- h.write("\n#ifdef __cplusplus\nextern \"C\" {\n#endif\n\n");
+ h.write("#ifndef __" + mName.toUpperCase().replace('.', '_') + "__\n");
+ h.write("#define __" + mName.toUpperCase().replace('.', '_') + "__\n");
- c.write("#include <stdlib.h>\n"); // need it for calloc() & free()
- c.write("#include \""+mName+".h\"\n\n");
+ h.write("#include \"recordio.h\"\n");
+ for (Iterator<JFile> i = mInclFiles.iterator(); i.hasNext(); ) {
+ JFile f = i.next();
+ h.write("#include \"" + f.getName() + ".h\"\n");
+ }
+ // required for compilation from C++
+ h.write("\n#ifdef __cplusplus\nextern \"C\" {\n#endif\n\n");
- for (Iterator<JRecord> i = mRecList.iterator(); i.hasNext();) {
- JRecord jr = i.next();
- jr.genCCode(h, c);
- }
+ c.write("#include <stdlib.h>\n"); // need it for calloc() & free()
+ c.write("#include \"" + mName + ".h\"\n\n");
- h.write("\n#ifdef __cplusplus\n}\n#endif\n\n");
- h.write("#endif //"+mName.toUpperCase().replace('.','_')+"__\n");
+ for (Iterator<JRecord> i = mRecList.iterator(); i.hasNext(); ) {
+ JRecord jr = i.next();
+ jr.genCCode(h, c);
+ }
- h.close();
- c.close();
+ h.write("\n#ifdef __cplusplus\n}\n#endif\n\n");
+ h.write("#endif //" + mName.toUpperCase().replace('.', '_') + "__\n");
+ } catch (IOException e) {
+ throw e;
+ } finally {
+ if (h != null) {
+ try {
+ h.close();
+ } catch (IOException ex) {
+ throw ex;
+ } finally {
+ if (c != null) {
+ c.close();
+ }
+ }
+ }
+ if (c != null) {
--- End diff --
It's sad that we have to close c twice (one here, one in the finally block earlier.). Findbugs is not happy if either of them is missing. It could be that the flow sensitive engine of findbug need an improvement.
---
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] zookeeper issue #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on the issue:
https://github.com/apache/zookeeper/pull/102
@fpj Thank you for taking time to double check the patch.
>> I noticed that there are also unaddressed comments from @lvfangmin and @eribeiro .
The comments were about using Interface type instead of implementation type, and I addressed the comments by creating ZOOKEEPER-2630 because this issue is not a regression and is not specifically about findbug warnings, and I think it might be better to address them separately.
I've left replies on your other comments in PR, please take a look.
---
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] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by eribeiro <gi...@git.apache.org>.
Github user eribeiro commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/102#discussion_r87027160
--- Diff: src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumHierarchical.java ---
@@ -344,9 +345,10 @@ public boolean containsQuorum(Set<Long> set){
* Check if all groups have majority
*/
int majGroupCounter = 0;
- for(long gid : expansion.keySet()) {
- LOG.debug("Group info: " + expansion.get(gid) + ", " + gid + ", " + groupWeight.get(gid));
- if(expansion.get(gid) > (groupWeight.get(gid) / 2) )
+ for (Entry<Long, Long> entry : expansion.entrySet()) {
+ Long gid = entry.getKey();
+ LOG.debug("Group info: " + entry.getValue() + ", " + gid + ", " + groupWeight.get(gid));
--- End diff --
We could use modern LOG printing here:
``
LOG.debug("Group info: {}, {}, {}", entry.getValue(), gid, groupWeight.get(gid));
``
---
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] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/102#discussion_r89355986
--- Diff: src/java/main/org/apache/jute/compiler/JRecord.java ---
@@ -403,168 +410,165 @@ public void genJavaCode(File outputDirectory) throws IOException {
} else if (!pkgdir.isDirectory()) {
throw new IOException(pkgpath + " is not a directory.");
}
- File jfile = new File(pkgdir, getName()+".java");
- FileWriter jj = new FileWriter(jfile);
- jj.write("// File generated by hadoop record compiler. Do not edit.\n");
- jj.write("/**\n");
- jj.write("* Licensed to the Apache Software Foundation (ASF) under one\n");
- jj.write("* or more contributor license agreements. See the NOTICE file\n");
- jj.write("* distributed with this work for additional information\n");
- jj.write("* regarding copyright ownership. The ASF licenses this file\n");
- jj.write("* to you under the Apache License, Version 2.0 (the\n");
- jj.write("* \"License\"); you may not use this file except in compliance\n");
- jj.write("* with the License. You may obtain a copy of the License at\n");
- jj.write("*\n");
- jj.write("* http://www.apache.org/licenses/LICENSE-2.0\n");
- jj.write("*\n");
- jj.write("* Unless required by applicable law or agreed to in writing, software\n");
- jj.write("* distributed under the License is distributed on an \"AS IS\" BASIS,\n");
- jj.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n");
- jj.write("* See the License for the specific language governing permissions and\n");
- jj.write("* limitations under the License.\n");
- jj.write("*/\n");
- jj.write("\n");
- jj.write("package "+getJavaPackage()+";\n\n");
- jj.write("import org.apache.jute.*;\n");
- jj.write("public class "+getName()+" implements Record {\n");
- for (Iterator<JField> i = mFields.iterator(); i.hasNext();) {
- JField jf = i.next();
- jj.write(jf.genJavaDecl());
- }
- jj.write(" public "+getName()+"() {\n");
- jj.write(" }\n");
+ try (FileWriter jj = new FileWriter(new File(pkgdir, getName()+".java"))) {
+ jj.write("// File generated by hadoop record compiler. Do not edit.\n");
+ jj.write("/**\n");
+ jj.write("* Licensed to the Apache Software Foundation (ASF) under one\n");
+ jj.write("* or more contributor license agreements. See the NOTICE file\n");
+ jj.write("* distributed with this work for additional information\n");
+ jj.write("* regarding copyright ownership. The ASF licenses this file\n");
+ jj.write("* to you under the Apache License, Version 2.0 (the\n");
+ jj.write("* \"License\"); you may not use this file except in compliance\n");
+ jj.write("* with the License. You may obtain a copy of the License at\n");
+ jj.write("*\n");
+ jj.write("* http://www.apache.org/licenses/LICENSE-2.0\n");
+ jj.write("*\n");
+ jj.write("* Unless required by applicable law or agreed to in writing, software\n");
+ jj.write("* distributed under the License is distributed on an \"AS IS\" BASIS,\n");
+ jj.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n");
+ jj.write("* See the License for the specific language governing permissions and\n");
+ jj.write("* limitations under the License.\n");
+ jj.write("*/\n");
+ jj.write("\n");
+ jj.write("package " + getJavaPackage() + ";\n\n");
+ jj.write("import org.apache.jute.*;\n");
+ jj.write("public class " + getName() + " implements Record {\n");
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); ) {
+ JField jf = i.next();
+ jj.write(jf.genJavaDecl());
+ }
+ jj.write(" public " + getName() + "() {\n");
+ jj.write(" }\n");
- jj.write(" public "+getName()+"(\n");
- int fIdx = 0;
- int fLen = mFields.size();
- for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
- JField jf = i.next();
- jj.write(jf.genJavaConstructorParam(jf.getName()));
- jj.write((fLen-1 == fIdx)?"":",\n");
- }
- jj.write(") {\n");
- fIdx = 0;
- for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
- JField jf = i.next();
- jj.write(jf.genJavaConstructorSet(jf.getName()));
- }
- jj.write(" }\n");
- fIdx = 0;
- for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
- JField jf = i.next();
- jj.write(jf.genJavaGetSet(fIdx));
- }
- jj.write(" public void serialize(OutputArchive a_, String tag) throws java.io.IOException {\n");
- jj.write(" a_.startRecord(this,tag);\n");
- fIdx = 0;
- for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
- JField jf = i.next();
- jj.write(jf.genJavaWriteMethodName());
- }
- jj.write(" a_.endRecord(this,tag);\n");
- jj.write(" }\n");
+ jj.write(" public " + getName() + "(\n");
+ int fIdx = 0;
+ int fLen = mFields.size();
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ jj.write(jf.genJavaConstructorParam(jf.getName()));
+ jj.write((fLen - 1 == fIdx) ? "" : ",\n");
+ }
+ jj.write(") {\n");
+ fIdx = 0;
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ jj.write(jf.genJavaConstructorSet(jf.getName()));
+ }
+ jj.write(" }\n");
+ fIdx = 0;
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ jj.write(jf.genJavaGetSet(fIdx));
+ }
+ jj.write(" public void serialize(OutputArchive a_, String tag) throws java.io.IOException {\n");
+ jj.write(" a_.startRecord(this,tag);\n");
+ fIdx = 0;
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ jj.write(jf.genJavaWriteMethodName());
+ }
+ jj.write(" a_.endRecord(this,tag);\n");
+ jj.write(" }\n");
- jj.write(" public void deserialize(InputArchive a_, String tag) throws java.io.IOException {\n");
- jj.write(" a_.startRecord(tag);\n");
- fIdx = 0;
- for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
- JField jf = i.next();
- jj.write(jf.genJavaReadMethodName());
- }
- jj.write(" a_.endRecord(tag);\n");
- jj.write("}\n");
-
- jj.write(" public String toString() {\n");
- jj.write(" try {\n");
- jj.write(" java.io.ByteArrayOutputStream s =\n");
- jj.write(" new java.io.ByteArrayOutputStream();\n");
- jj.write(" CsvOutputArchive a_ = \n");
- jj.write(" new CsvOutputArchive(s);\n");
- jj.write(" a_.startRecord(this,\"\");\n");
- fIdx = 0;
- for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
- JField jf = i.next();
- jj.write(jf.genJavaWriteMethodName());
- }
- jj.write(" a_.endRecord(this,\"\");\n");
- jj.write(" return new String(s.toByteArray(), \"UTF-8\");\n");
- jj.write(" } catch (Throwable ex) {\n");
- jj.write(" ex.printStackTrace();\n");
- jj.write(" }\n");
- jj.write(" return \"ERROR\";\n");
- jj.write(" }\n");
-
- jj.write(" public void write(java.io.DataOutput out) throws java.io.IOException {\n");
- jj.write(" BinaryOutputArchive archive = new BinaryOutputArchive(out);\n");
- jj.write(" serialize(archive, \"\");\n");
- jj.write(" }\n");
-
- jj.write(" public void readFields(java.io.DataInput in) throws java.io.IOException {\n");
- jj.write(" BinaryInputArchive archive = new BinaryInputArchive(in);\n");
- jj.write(" deserialize(archive, \"\");\n");
- jj.write(" }\n");
-
- jj.write(" public int compareTo (Object peer_) throws ClassCastException {\n");
- boolean unimplemented = false;
- for (JField f : mFields) {
- if ((f.getType() instanceof JMap)
- || (f.getType() instanceof JVector))
- {
- unimplemented = true;
+ jj.write(" public void deserialize(InputArchive a_, String tag) throws java.io.IOException {\n");
+ jj.write(" a_.startRecord(tag);\n");
+ fIdx = 0;
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ jj.write(jf.genJavaReadMethodName());
}
- }
- if (unimplemented) {
- jj.write(" throw new UnsupportedOperationException(\"comparing "
- + getName() + " is unimplemented\");\n");
- } else {
- jj.write(" if (!(peer_ instanceof "+getName()+")) {\n");
- jj.write(" throw new ClassCastException(\"Comparing different types of records.\");\n");
+ jj.write(" a_.endRecord(tag);\n");
+ jj.write("}\n");
+
+ jj.write(" public String toString() {\n");
+ jj.write(" try {\n");
+ jj.write(" java.io.ByteArrayOutputStream s =\n");
+ jj.write(" new java.io.ByteArrayOutputStream();\n");
+ jj.write(" CsvOutputArchive a_ = \n");
+ jj.write(" new CsvOutputArchive(s);\n");
+ jj.write(" a_.startRecord(this,\"\");\n");
+ fIdx = 0;
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ jj.write(jf.genJavaWriteMethodName());
+ }
+ jj.write(" a_.endRecord(this,\"\");\n");
+ jj.write(" return new String(s.toByteArray(), \"UTF-8\");\n");
+ jj.write(" } catch (Throwable ex) {\n");
+ jj.write(" ex.printStackTrace();\n");
jj.write(" }\n");
- jj.write(" "+getName()+" peer = ("+getName()+") peer_;\n");
- jj.write(" int ret = 0;\n");
+ jj.write(" return \"ERROR\";\n");
+ jj.write(" }\n");
+
+ jj.write(" public void write(java.io.DataOutput out) throws java.io.IOException {\n");
+ jj.write(" BinaryOutputArchive archive = new BinaryOutputArchive(out);\n");
+ jj.write(" serialize(archive, \"\");\n");
+ jj.write(" }\n");
+
+ jj.write(" public void readFields(java.io.DataInput in) throws java.io.IOException {\n");
+ jj.write(" BinaryInputArchive archive = new BinaryInputArchive(in);\n");
+ jj.write(" deserialize(archive, \"\");\n");
+ jj.write(" }\n");
+
+ jj.write(" public int compareTo (Object peer_) throws ClassCastException {\n");
+ boolean unimplemented = false;
+ for (JField f : mFields) {
+ if ((f.getType() instanceof JMap)
+ || (f.getType() instanceof JVector)) {
+ unimplemented = true;
+ }
+ }
+ if (unimplemented) {
+ jj.write(" throw new UnsupportedOperationException(\"comparing "
+ + getName() + " is unimplemented\");\n");
+ } else {
+ jj.write(" if (!(peer_ instanceof " + getName() + ")) {\n");
+ jj.write(" throw new ClassCastException(\"Comparing different types of records.\");\n");
+ jj.write(" }\n");
+ jj.write(" " + getName() + " peer = (" + getName() + ") peer_;\n");
+ jj.write(" int ret = 0;\n");
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ jj.write(jf.genJavaCompareTo());
+ jj.write(" if (ret != 0) return ret;\n");
+ }
+ jj.write(" return ret;\n");
+ }
+ jj.write(" }\n");
+
+ jj.write(" public boolean equals(Object peer_) {\n");
+ jj.write(" if (!(peer_ instanceof " + getName() + ")) {\n");
+ jj.write(" return false;\n");
+ jj.write(" }\n");
+ jj.write(" if (peer_ == this) {\n");
+ jj.write(" return true;\n");
+ jj.write(" }\n");
+ jj.write(" " + getName() + " peer = (" + getName() + ") peer_;\n");
+ jj.write(" boolean ret = false;\n");
for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
JField jf = i.next();
- jj.write(jf.genJavaCompareTo());
- jj.write(" if (ret != 0) return ret;\n");
+ jj.write(jf.genJavaEquals());
--- End diff --
Similarly, there is no actual change here.
---
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] zookeeper issue #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on the issue:
https://github.com/apache/zookeeper/pull/102
Update: disable "Malicious code vulnerability Warnings" appertains to org.apache.zookeeper.ZooDefs$Ids.OPEN_ACL_UNSAFE, and we will use ZOOKEEPER-1362 for this work.
Can we get this merged? Would be good to have findbug clean again for pre-commit builds.
---
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] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by eribeiro <gi...@git.apache.org>.
Github user eribeiro commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/102#discussion_r87038798
--- Diff: src/java/main/org/apache/jute/compiler/JRecord.java ---
@@ -141,109 +141,128 @@ public String genCsharpWriteWrapper(String fname, String tag) {
static HashMap<String, String> vectorStructs = new HashMap<String, String>();
public void genCCode(FileWriter h, FileWriter c) throws IOException {
- for (JField f : mFields) {
- if (f.getType() instanceof JVector) {
- JVector jv = (JVector)f.getType();
- JType jvType = jv.getElementType();
- String struct_name = JVector.extractVectorName(jvType);
- if (vectorStructs.get(struct_name) == null) {
- vectorStructs.put(struct_name, struct_name);
- h.write("struct " + struct_name + " {\n int32_t count;\n" + jv.getElementType().genCDecl("*data") + "\n};\n");
- h.write("int serialize_" + struct_name + "(struct oarchive *out, const char *tag, struct " + struct_name + " *v);\n");
- h.write("int deserialize_" + struct_name + "(struct iarchive *in, const char *tag, struct " + struct_name + " *v);\n");
- h.write("int allocate_" + struct_name + "(struct " + struct_name + " *v, int32_t len);\n");
- h.write("int deallocate_" + struct_name + "(struct " + struct_name + " *v);\n");
- c.write("int allocate_" + struct_name + "(struct " + struct_name + " *v, int32_t len) {\n");
- c.write(" if (!len) {\n");
- c.write(" v->count = 0;\n");
- c.write(" v->data = 0;\n");
- c.write(" } else {\n");
- c.write(" v->count = len;\n");
- c.write(" v->data = calloc(sizeof(*v->data), len);\n");
- c.write(" }\n");
- c.write(" return 0;\n");
- c.write("}\n");
- c.write("int deallocate_" + struct_name + "(struct " + struct_name + " *v) {\n");
- c.write(" if (v->data) {\n");
- c.write(" int32_t i;\n");
- c.write(" for(i=0;i<v->count; i++) {\n");
- c.write(" deallocate_"+JRecord.extractMethodSuffix(jvType)+"(&v->data[i]);\n");
- c.write(" }\n");
- c.write(" free(v->data);\n");
- c.write(" v->data = 0;\n");
- c.write(" }\n");
- c.write(" return 0;\n");
- c.write("}\n");
- c.write("int serialize_" + struct_name + "(struct oarchive *out, const char *tag, struct " + struct_name + " *v)\n");
- c.write("{\n");
- c.write(" int32_t count = v->count;\n");
- c.write(" int rc = 0;\n");
- c.write(" int32_t i;\n");
- c.write(" rc = out->start_vector(out, tag, &count);\n");
- c.write(" for(i=0;i<v->count;i++) {\n");
- genSerialize(c, jvType, "data", "data[i]");
- c.write(" }\n");
- c.write(" rc = rc ? rc : out->end_vector(out, tag);\n");
- c.write(" return rc;\n");
- c.write("}\n");
- c.write("int deserialize_" + struct_name + "(struct iarchive *in, const char *tag, struct " + struct_name + " *v)\n");
- c.write("{\n");
- c.write(" int rc = 0;\n");
- c.write(" int32_t i;\n");
- c.write(" rc = in->start_vector(in, tag, &v->count);\n");
- c.write(" v->data = calloc(v->count, sizeof(*v->data));\n");
- c.write(" for(i=0;i<v->count;i++) {\n");
- genDeserialize(c, jvType, "value", "data[i]");
- c.write(" }\n");
- c.write(" rc = in->end_vector(in, tag);\n");
- c.write(" return rc;\n");
- c.write("}\n");
-
+ try {
+ for (JField f : mFields) {
+ if (f.getType() instanceof JVector) {
+ JVector jv = (JVector) f.getType();
+ JType jvType = jv.getElementType();
+ String struct_name = JVector.extractVectorName(jvType);
+ if (vectorStructs.get(struct_name) == null) {
+ vectorStructs.put(struct_name, struct_name);
+ h.write("struct " + struct_name + " {\n int32_t count;\n" + jv.getElementType().genCDecl("*data") + "\n};\n");
+ h.write("int serialize_" + struct_name + "(struct oarchive *out, const char *tag, struct " + struct_name + " *v);\n");
+ h.write("int deserialize_" + struct_name + "(struct iarchive *in, const char *tag, struct " + struct_name + " *v);\n");
+ h.write("int allocate_" + struct_name + "(struct " + struct_name + " *v, int32_t len);\n");
+ h.write("int deallocate_" + struct_name + "(struct " + struct_name + " *v);\n");
+ c.write("int allocate_" + struct_name + "(struct " + struct_name + " *v, int32_t len) {\n");
+ c.write(" if (!len) {\n");
+ c.write(" v->count = 0;\n");
+ c.write(" v->data = 0;\n");
+ c.write(" } else {\n");
+ c.write(" v->count = len;\n");
+ c.write(" v->data = calloc(sizeof(*v->data), len);\n");
+ c.write(" }\n");
+ c.write(" return 0;\n");
+ c.write("}\n");
+ c.write("int deallocate_" + struct_name + "(struct " + struct_name + " *v) {\n");
+ c.write(" if (v->data) {\n");
+ c.write(" int32_t i;\n");
+ c.write(" for(i=0;i<v->count; i++) {\n");
+ c.write(" deallocate_" + JRecord.extractMethodSuffix(jvType) + "(&v->data[i]);\n");
+ c.write(" }\n");
+ c.write(" free(v->data);\n");
+ c.write(" v->data = 0;\n");
+ c.write(" }\n");
+ c.write(" return 0;\n");
+ c.write("}\n");
+ c.write("int serialize_" + struct_name + "(struct oarchive *out, const char *tag, struct " + struct_name + " *v)\n");
+ c.write("{\n");
+ c.write(" int32_t count = v->count;\n");
+ c.write(" int rc = 0;\n");
+ c.write(" int32_t i;\n");
+ c.write(" rc = out->start_vector(out, tag, &count);\n");
+ c.write(" for(i=0;i<v->count;i++) {\n");
+ genSerialize(c, jvType, "data", "data[i]");
+ c.write(" }\n");
+ c.write(" rc = rc ? rc : out->end_vector(out, tag);\n");
+ c.write(" return rc;\n");
+ c.write("}\n");
+ c.write("int deserialize_" + struct_name + "(struct iarchive *in, const char *tag, struct " + struct_name + " *v)\n");
+ c.write("{\n");
+ c.write(" int rc = 0;\n");
+ c.write(" int32_t i;\n");
+ c.write(" rc = in->start_vector(in, tag, &v->count);\n");
+ c.write(" v->data = calloc(v->count, sizeof(*v->data));\n");
+ c.write(" for(i=0;i<v->count;i++) {\n");
+ genDeserialize(c, jvType, "value", "data[i]");
+ c.write(" }\n");
+ c.write(" rc = in->end_vector(in, tag);\n");
+ c.write(" return rc;\n");
+ c.write("}\n");
+
+ }
}
}
- }
- String rec_name = getName();
- h.write("struct " + rec_name + " {\n");
- for (JField f : mFields) {
- h.write(f.genCDecl());
- }
- h.write("};\n");
- h.write("int serialize_" + rec_name + "(struct oarchive *out, const char *tag, struct " + rec_name + " *v);\n");
- h.write("int deserialize_" + rec_name + "(struct iarchive *in, const char *tag, struct " + rec_name + "*v);\n");
- h.write("void deallocate_" + rec_name + "(struct " + rec_name + "*);\n");
- c.write("int serialize_" + rec_name + "(struct oarchive *out, const char *tag, struct " + rec_name + " *v)");
- c.write("{\n");
- c.write(" int rc;\n");
- c.write(" rc = out->start_record(out, tag);\n");
- for(JField f : mFields) {
- genSerialize(c, f.getType(), f.getTag(), f.getName());
- }
- c.write(" rc = rc ? rc : out->end_record(out, tag);\n");
- c.write(" return rc;\n");
- c.write("}\n");
- c.write("int deserialize_" + rec_name + "(struct iarchive *in, const char *tag, struct " + rec_name + "*v)");
- c.write("{\n");
- c.write(" int rc;\n");
- c.write(" rc = in->start_record(in, tag);\n");
- for(JField f : mFields) {
- genDeserialize(c, f.getType(), f.getTag(), f.getName());
- }
- c.write(" rc = rc ? rc : in->end_record(in, tag);\n");
- c.write(" return rc;\n");
- c.write("}\n");
- c.write("void deallocate_" + rec_name + "(struct " + rec_name + "*v)");
- c.write("{\n");
- for(JField f : mFields) {
- if (f.getType() instanceof JRecord) {
- c.write(" deallocate_" + extractStructName(f.getType()) + "(&v->" + f.getName() + ");\n");
- } else if (f.getType() instanceof JVector) {
- JVector vt = (JVector)f.getType();
- c.write(" deallocate_" + JVector.extractVectorName(vt.getElementType())+ "(&v->"+f.getName()+");\n");
- } else if (f.getType() instanceof JCompType) {
- c.write(" deallocate_" + extractMethodSuffix(f.getType()) + "(&v->"+f.getName()+");\n");
+ String rec_name = getName();
+ h.write("struct " + rec_name + " {\n");
+ for (JField f : mFields) {
+ h.write(f.genCDecl());
+ }
+ h.write("};\n");
+ h.write("int serialize_" + rec_name + "(struct oarchive *out, const char *tag, struct " + rec_name + " *v);\n");
+ h.write("int deserialize_" + rec_name + "(struct iarchive *in, const char *tag, struct " + rec_name + "*v);\n");
+ h.write("void deallocate_" + rec_name + "(struct " + rec_name + "*);\n");
+ c.write("int serialize_" + rec_name + "(struct oarchive *out, const char *tag, struct " + rec_name + " *v)");
+ c.write("{\n");
+ c.write(" int rc;\n");
+ c.write(" rc = out->start_record(out, tag);\n");
+ for (JField f : mFields) {
+ genSerialize(c, f.getType(), f.getTag(), f.getName());
+ }
+ c.write(" rc = rc ? rc : out->end_record(out, tag);\n");
+ c.write(" return rc;\n");
+ c.write("}\n");
+ c.write("int deserialize_" + rec_name + "(struct iarchive *in, const char *tag, struct " + rec_name + "*v)");
+ c.write("{\n");
+ c.write(" int rc;\n");
+ c.write(" rc = in->start_record(in, tag);\n");
+ for (JField f : mFields) {
+ genDeserialize(c, f.getType(), f.getTag(), f.getName());
+ }
+ c.write(" rc = rc ? rc : in->end_record(in, tag);\n");
+ c.write(" return rc;\n");
+ c.write("}\n");
+ c.write("void deallocate_" + rec_name + "(struct " + rec_name + "*v)");
+ c.write("{\n");
+ for (JField f : mFields) {
+ if (f.getType() instanceof JRecord) {
+ c.write(" deallocate_" + extractStructName(f.getType()) + "(&v->" + f.getName() + ");\n");
+ } else if (f.getType() instanceof JVector) {
+ JVector vt = (JVector) f.getType();
+ c.write(" deallocate_" + JVector.extractVectorName(vt.getElementType()) + "(&v->" + f.getName() + ");\n");
+ } else if (f.getType() instanceof JCompType) {
+ c.write(" deallocate_" + extractMethodSuffix(f.getType()) + "(&v->" + f.getName() + ");\n");
+ }
+ }
+ c.write("}\n");
+ } catch (IOException e) {
+ throw e;
+ } finally {
+ if (h != null) {
--- End diff --
Just speculating here: couldn't we write this snippet as:
```
private IOException maybeClose(FileWriter file) {
IOException t = null;
if (file != null) {
try {
file.close();
} catch (IOException ex) {
t = ex;
}
}
return t;
}
(...)
} finally {
IOException e1 = maybeClose(h);
IOException e2 = maybeClose(c);
if (e1 != null) {
throw e1;
}
if (e2 != null) {
throw e2;
}
}
```
---
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] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by eribeiro <gi...@git.apache.org>.
Github user eribeiro commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/102#discussion_r87026194
--- Diff: src/java/main/org/apache/zookeeper/version/util/VerGen.java ---
@@ -50,10 +50,8 @@ public static void generateFile(File outputDir, Version version, int rev, String
System.out.println(path + " is not a directory.");
System.exit(1);
}
- File file = new File(pkgdir, TYPE_NAME + ".java");
- FileWriter w = null;
- try {
- w = new FileWriter(file);
+
+ try (FileWriter w = new FileWriter(new File(pkgdir, TYPE_NAME + ".java"));) {
--- End diff --
nit: spurious ";"
---
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] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by fpj <gi...@git.apache.org>.
Github user fpj commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/102#discussion_r89283922
--- Diff: src/java/main/org/apache/zookeeper/server/command/FourLetterCommands.java ---
@@ -151,9 +153,13 @@
*/
public final static int telnetCloseCmd = 0xfff4fffd;
- public final static HashMap<Integer, String> cmd2String =
+ final static HashMap<Integer, String> cmd2String =
--- End diff --
If we can remove this `public`, then I think we should. Also agree with the consistent `Map` declaration comment.
---
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] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by lvfangmin <gi...@git.apache.org>.
Github user lvfangmin commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/102#discussion_r87265113
--- Diff: src/java/main/org/apache/zookeeper/server/command/FourLetterCommands.java ---
@@ -151,9 +153,13 @@
*/
public final static int telnetCloseCmd = 0xfff4fffd;
- public final static HashMap<Integer, String> cmd2String =
+ final static HashMap<Integer, String> cmd2String =
--- End diff --
Same here, we can also define this as Map to keep consistent with the others.
---
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] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by fpj <gi...@git.apache.org>.
Github user fpj commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/102#discussion_r89264869
--- Diff: src/java/main/org/apache/jute/compiler/JRecord.java ---
@@ -403,168 +410,165 @@ public void genJavaCode(File outputDirectory) throws IOException {
} else if (!pkgdir.isDirectory()) {
throw new IOException(pkgpath + " is not a directory.");
}
- File jfile = new File(pkgdir, getName()+".java");
- FileWriter jj = new FileWriter(jfile);
- jj.write("// File generated by hadoop record compiler. Do not edit.\n");
- jj.write("/**\n");
- jj.write("* Licensed to the Apache Software Foundation (ASF) under one\n");
- jj.write("* or more contributor license agreements. See the NOTICE file\n");
- jj.write("* distributed with this work for additional information\n");
- jj.write("* regarding copyright ownership. The ASF licenses this file\n");
- jj.write("* to you under the Apache License, Version 2.0 (the\n");
- jj.write("* \"License\"); you may not use this file except in compliance\n");
- jj.write("* with the License. You may obtain a copy of the License at\n");
- jj.write("*\n");
- jj.write("* http://www.apache.org/licenses/LICENSE-2.0\n");
- jj.write("*\n");
- jj.write("* Unless required by applicable law or agreed to in writing, software\n");
- jj.write("* distributed under the License is distributed on an \"AS IS\" BASIS,\n");
- jj.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n");
- jj.write("* See the License for the specific language governing permissions and\n");
- jj.write("* limitations under the License.\n");
- jj.write("*/\n");
- jj.write("\n");
- jj.write("package "+getJavaPackage()+";\n\n");
- jj.write("import org.apache.jute.*;\n");
- jj.write("public class "+getName()+" implements Record {\n");
- for (Iterator<JField> i = mFields.iterator(); i.hasNext();) {
- JField jf = i.next();
- jj.write(jf.genJavaDecl());
- }
- jj.write(" public "+getName()+"() {\n");
- jj.write(" }\n");
+ try (FileWriter jj = new FileWriter(new File(pkgdir, getName()+".java"))) {
+ jj.write("// File generated by hadoop record compiler. Do not edit.\n");
+ jj.write("/**\n");
+ jj.write("* Licensed to the Apache Software Foundation (ASF) under one\n");
+ jj.write("* or more contributor license agreements. See the NOTICE file\n");
+ jj.write("* distributed with this work for additional information\n");
+ jj.write("* regarding copyright ownership. The ASF licenses this file\n");
+ jj.write("* to you under the Apache License, Version 2.0 (the\n");
+ jj.write("* \"License\"); you may not use this file except in compliance\n");
+ jj.write("* with the License. You may obtain a copy of the License at\n");
+ jj.write("*\n");
+ jj.write("* http://www.apache.org/licenses/LICENSE-2.0\n");
+ jj.write("*\n");
+ jj.write("* Unless required by applicable law or agreed to in writing, software\n");
+ jj.write("* distributed under the License is distributed on an \"AS IS\" BASIS,\n");
+ jj.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n");
+ jj.write("* See the License for the specific language governing permissions and\n");
+ jj.write("* limitations under the License.\n");
+ jj.write("*/\n");
+ jj.write("\n");
+ jj.write("package " + getJavaPackage() + ";\n\n");
+ jj.write("import org.apache.jute.*;\n");
+ jj.write("public class " + getName() + " implements Record {\n");
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); ) {
+ JField jf = i.next();
+ jj.write(jf.genJavaDecl());
+ }
+ jj.write(" public " + getName() + "() {\n");
+ jj.write(" }\n");
- jj.write(" public "+getName()+"(\n");
- int fIdx = 0;
- int fLen = mFields.size();
- for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
- JField jf = i.next();
- jj.write(jf.genJavaConstructorParam(jf.getName()));
- jj.write((fLen-1 == fIdx)?"":",\n");
- }
- jj.write(") {\n");
- fIdx = 0;
- for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
- JField jf = i.next();
- jj.write(jf.genJavaConstructorSet(jf.getName()));
- }
- jj.write(" }\n");
- fIdx = 0;
- for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
- JField jf = i.next();
- jj.write(jf.genJavaGetSet(fIdx));
- }
- jj.write(" public void serialize(OutputArchive a_, String tag) throws java.io.IOException {\n");
- jj.write(" a_.startRecord(this,tag);\n");
- fIdx = 0;
- for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
- JField jf = i.next();
- jj.write(jf.genJavaWriteMethodName());
- }
- jj.write(" a_.endRecord(this,tag);\n");
- jj.write(" }\n");
+ jj.write(" public " + getName() + "(\n");
+ int fIdx = 0;
+ int fLen = mFields.size();
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ jj.write(jf.genJavaConstructorParam(jf.getName()));
+ jj.write((fLen - 1 == fIdx) ? "" : ",\n");
+ }
+ jj.write(") {\n");
+ fIdx = 0;
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ jj.write(jf.genJavaConstructorSet(jf.getName()));
+ }
+ jj.write(" }\n");
+ fIdx = 0;
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ jj.write(jf.genJavaGetSet(fIdx));
+ }
+ jj.write(" public void serialize(OutputArchive a_, String tag) throws java.io.IOException {\n");
+ jj.write(" a_.startRecord(this,tag);\n");
+ fIdx = 0;
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ jj.write(jf.genJavaWriteMethodName());
+ }
+ jj.write(" a_.endRecord(this,tag);\n");
+ jj.write(" }\n");
- jj.write(" public void deserialize(InputArchive a_, String tag) throws java.io.IOException {\n");
- jj.write(" a_.startRecord(tag);\n");
- fIdx = 0;
- for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
- JField jf = i.next();
- jj.write(jf.genJavaReadMethodName());
- }
- jj.write(" a_.endRecord(tag);\n");
- jj.write("}\n");
-
- jj.write(" public String toString() {\n");
- jj.write(" try {\n");
- jj.write(" java.io.ByteArrayOutputStream s =\n");
- jj.write(" new java.io.ByteArrayOutputStream();\n");
- jj.write(" CsvOutputArchive a_ = \n");
- jj.write(" new CsvOutputArchive(s);\n");
- jj.write(" a_.startRecord(this,\"\");\n");
- fIdx = 0;
- for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
- JField jf = i.next();
- jj.write(jf.genJavaWriteMethodName());
- }
- jj.write(" a_.endRecord(this,\"\");\n");
- jj.write(" return new String(s.toByteArray(), \"UTF-8\");\n");
- jj.write(" } catch (Throwable ex) {\n");
- jj.write(" ex.printStackTrace();\n");
- jj.write(" }\n");
- jj.write(" return \"ERROR\";\n");
- jj.write(" }\n");
-
- jj.write(" public void write(java.io.DataOutput out) throws java.io.IOException {\n");
- jj.write(" BinaryOutputArchive archive = new BinaryOutputArchive(out);\n");
- jj.write(" serialize(archive, \"\");\n");
- jj.write(" }\n");
-
- jj.write(" public void readFields(java.io.DataInput in) throws java.io.IOException {\n");
- jj.write(" BinaryInputArchive archive = new BinaryInputArchive(in);\n");
- jj.write(" deserialize(archive, \"\");\n");
- jj.write(" }\n");
-
- jj.write(" public int compareTo (Object peer_) throws ClassCastException {\n");
- boolean unimplemented = false;
- for (JField f : mFields) {
- if ((f.getType() instanceof JMap)
- || (f.getType() instanceof JVector))
- {
- unimplemented = true;
+ jj.write(" public void deserialize(InputArchive a_, String tag) throws java.io.IOException {\n");
+ jj.write(" a_.startRecord(tag);\n");
+ fIdx = 0;
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ jj.write(jf.genJavaReadMethodName());
}
- }
- if (unimplemented) {
- jj.write(" throw new UnsupportedOperationException(\"comparing "
- + getName() + " is unimplemented\");\n");
- } else {
- jj.write(" if (!(peer_ instanceof "+getName()+")) {\n");
- jj.write(" throw new ClassCastException(\"Comparing different types of records.\");\n");
+ jj.write(" a_.endRecord(tag);\n");
+ jj.write("}\n");
+
+ jj.write(" public String toString() {\n");
+ jj.write(" try {\n");
+ jj.write(" java.io.ByteArrayOutputStream s =\n");
+ jj.write(" new java.io.ByteArrayOutputStream();\n");
+ jj.write(" CsvOutputArchive a_ = \n");
+ jj.write(" new CsvOutputArchive(s);\n");
+ jj.write(" a_.startRecord(this,\"\");\n");
+ fIdx = 0;
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ jj.write(jf.genJavaWriteMethodName());
+ }
+ jj.write(" a_.endRecord(this,\"\");\n");
+ jj.write(" return new String(s.toByteArray(), \"UTF-8\");\n");
+ jj.write(" } catch (Throwable ex) {\n");
+ jj.write(" ex.printStackTrace();\n");
jj.write(" }\n");
- jj.write(" "+getName()+" peer = ("+getName()+") peer_;\n");
- jj.write(" int ret = 0;\n");
+ jj.write(" return \"ERROR\";\n");
+ jj.write(" }\n");
+
+ jj.write(" public void write(java.io.DataOutput out) throws java.io.IOException {\n");
+ jj.write(" BinaryOutputArchive archive = new BinaryOutputArchive(out);\n");
+ jj.write(" serialize(archive, \"\");\n");
+ jj.write(" }\n");
+
+ jj.write(" public void readFields(java.io.DataInput in) throws java.io.IOException {\n");
+ jj.write(" BinaryInputArchive archive = new BinaryInputArchive(in);\n");
+ jj.write(" deserialize(archive, \"\");\n");
+ jj.write(" }\n");
+
+ jj.write(" public int compareTo (Object peer_) throws ClassCastException {\n");
+ boolean unimplemented = false;
+ for (JField f : mFields) {
+ if ((f.getType() instanceof JMap)
+ || (f.getType() instanceof JVector)) {
+ unimplemented = true;
+ }
+ }
+ if (unimplemented) {
+ jj.write(" throw new UnsupportedOperationException(\"comparing "
+ + getName() + " is unimplemented\");\n");
+ } else {
+ jj.write(" if (!(peer_ instanceof " + getName() + ")) {\n");
+ jj.write(" throw new ClassCastException(\"Comparing different types of records.\");\n");
+ jj.write(" }\n");
+ jj.write(" " + getName() + " peer = (" + getName() + ") peer_;\n");
+ jj.write(" int ret = 0;\n");
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ jj.write(jf.genJavaCompareTo());
--- End diff --
Why are we changing to `genJavaEquals` below and not here?
---
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] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by fpj <gi...@git.apache.org>.
Github user fpj commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/102#discussion_r89285632
--- Diff: src/java/main/org/apache/jute/compiler/JRecord.java ---
@@ -576,174 +580,174 @@ public void genCsharpCode(File outputDirectory) throws IOException {
} else if (!outputDirectory.isDirectory()) {
throw new IOException(outputDirectory + " is not a directory.");
}
- File csharpFile = new File(outputDirectory, getName()+".cs");
- FileWriter cs = new FileWriter(csharpFile);
- cs.write("// File generated by hadoop record compiler. Do not edit.\n");
- cs.write("/**\n");
- cs.write("* Licensed to the Apache Software Foundation (ASF) under one\n");
- cs.write("* or more contributor license agreements. See the NOTICE file\n");
- cs.write("* distributed with this work for additional information\n");
- cs.write("* regarding copyright ownership. The ASF licenses this file\n");
- cs.write("* to you under the Apache License, Version 2.0 (the\n");
- cs.write("* \"License\"); you may not use this file except in compliance\n");
- cs.write("* with the License. You may obtain a copy of the License at\n");
- cs.write("*\n");
- cs.write("* http://www.apache.org/licenses/LICENSE-2.0\n");
- cs.write("*\n");
- cs.write("* Unless required by applicable law or agreed to in writing, software\n");
- cs.write("* distributed under the License is distributed on an \"AS IS\" BASIS,\n");
- cs.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n");
- cs.write("* See the License for the specific language governing permissions and\n");
- cs.write("* limitations under the License.\n");
- cs.write("*/\n");
- cs.write("\n");
- cs.write("using System;\n");
- cs.write("using Org.Apache.Jute;\n");
- cs.write("\n");
- cs.write("namespace "+getCsharpNameSpace()+"\n");
- cs.write("{\n");
-
- String className = getCsharpName();
- cs.write("public class "+className+" : IRecord, IComparable \n");
- cs.write("{\n");
- cs.write(" public "+ className +"() {\n");
- cs.write(" }\n");
-
- cs.write(" public "+className+"(\n");
- int fIdx = 0;
- int fLen = mFields.size();
- for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
- JField jf = i.next();
- cs.write(jf.genCsharpConstructorParam(jf.getCsharpName()));
- cs.write((fLen-1 == fIdx)?"":",\n");
- }
- cs.write(") {\n");
- fIdx = 0;
- for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
- JField jf = i.next();
- cs.write(jf.genCsharpConstructorSet(jf.getCsharpName()));
- }
- cs.write(" }\n");
- fIdx = 0;
- for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
- JField jf = i.next();
- cs.write(jf.genCsharpGetSet(fIdx));
+
+ try (FileWriter cs = new FileWriter(new File(outputDirectory, getName() + ".cs"));) {
+ cs.write("// File generated by hadoop record compiler. Do not edit.\n");
+ cs.write("/**\n");
+ cs.write("* Licensed to the Apache Software Foundation (ASF) under one\n");
+ cs.write("* or more contributor license agreements. See the NOTICE file\n");
+ cs.write("* distributed with this work for additional information\n");
+ cs.write("* regarding copyright ownership. The ASF licenses this file\n");
+ cs.write("* to you under the Apache License, Version 2.0 (the\n");
+ cs.write("* \"License\"); you may not use this file except in compliance\n");
+ cs.write("* with the License. You may obtain a copy of the License at\n");
+ cs.write("*\n");
+ cs.write("* http://www.apache.org/licenses/LICENSE-2.0\n");
+ cs.write("*\n");
+ cs.write("* Unless required by applicable law or agreed to in writing, software\n");
+ cs.write("* distributed under the License is distributed on an \"AS IS\" BASIS,\n");
+ cs.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n");
+ cs.write("* See the License for the specific language governing permissions and\n");
+ cs.write("* limitations under the License.\n");
+ cs.write("*/\n");
cs.write("\n");
- }
- cs.write(" public void Serialize(IOutputArchive a_, String tag) {\n");
- cs.write(" a_.StartRecord(this,tag);\n");
- fIdx = 0;
- for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
- JField jf = i.next();
- cs.write(jf.genCsharpWriteMethodName());
- }
- cs.write(" a_.EndRecord(this,tag);\n");
- cs.write(" }\n");
+ cs.write("using System;\n");
+ cs.write("using Org.Apache.Jute;\n");
+ cs.write("\n");
+ cs.write("namespace " + getCsharpNameSpace() + "\n");
+ cs.write("{\n");
+
+ String className = getCsharpName();
+ cs.write("public class " + className + " : IRecord, IComparable \n");
+ cs.write("{\n");
+ cs.write(" public " + className + "() {\n");
+ cs.write(" }\n");
+
+ cs.write(" public " + className + "(\n");
+ int fIdx = 0;
+ int fLen = mFields.size();
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ cs.write(jf.genCsharpConstructorParam(jf.getCsharpName()));
+ cs.write((fLen - 1 == fIdx) ? "" : ",\n");
+ }
+ cs.write(") {\n");
+ fIdx = 0;
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ cs.write(jf.genCsharpConstructorSet(jf.getCsharpName()));
+ }
+ cs.write(" }\n");
+ fIdx = 0;
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ cs.write(jf.genCsharpGetSet(fIdx));
+ cs.write("\n");
+ }
+ cs.write(" public void Serialize(IOutputArchive a_, String tag) {\n");
+ cs.write(" a_.StartRecord(this,tag);\n");
+ fIdx = 0;
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ cs.write(jf.genCsharpWriteMethodName());
+ }
+ cs.write(" a_.EndRecord(this,tag);\n");
+ cs.write(" }\n");
- cs.write(" public void Deserialize(IInputArchive a_, String tag) {\n");
- cs.write(" a_.StartRecord(tag);\n");
- fIdx = 0;
- for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
- JField jf = i.next();
- cs.write(jf.genCsharpReadMethodName());
- }
- cs.write(" a_.EndRecord(tag);\n");
- cs.write("}\n");
-
- cs.write(" public override String ToString() {\n");
- cs.write(" try {\n");
- cs.write(" System.IO.MemoryStream ms = new System.IO.MemoryStream();\n");
- cs.write(" MiscUtil.IO.EndianBinaryWriter writer =\n");
- cs.write(" new MiscUtil.IO.EndianBinaryWriter(MiscUtil.Conversion.EndianBitConverter.Big, ms, System.Text.Encoding.UTF8);\n");
- cs.write(" BinaryOutputArchive a_ = \n");
- cs.write(" new BinaryOutputArchive(writer);\n");
- cs.write(" a_.StartRecord(this,\"\");\n");
- fIdx = 0;
- for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
- JField jf = i.next();
- cs.write(jf.genCsharpWriteMethodName());
- }
- cs.write(" a_.EndRecord(this,\"\");\n");
- cs.write(" ms.Position = 0;\n");
- cs.write(" return System.Text.Encoding.UTF8.GetString(ms.ToArray());\n");
- cs.write(" } catch (Exception ex) {\n");
- cs.write(" Console.WriteLine(ex.StackTrace);\n");
- cs.write(" }\n");
- cs.write(" return \"ERROR\";\n");
- cs.write(" }\n");
-
- cs.write(" public void Write(MiscUtil.IO.EndianBinaryWriter writer) {\n");
- cs.write(" BinaryOutputArchive archive = new BinaryOutputArchive(writer);\n");
- cs.write(" Serialize(archive, \"\");\n");
- cs.write(" }\n");
-
- cs.write(" public void ReadFields(MiscUtil.IO.EndianBinaryReader reader) {\n");
- cs.write(" BinaryInputArchive archive = new BinaryInputArchive(reader);\n");
- cs.write(" Deserialize(archive, \"\");\n");
- cs.write(" }\n");
-
- cs.write(" public int CompareTo (object peer_) {\n");
- boolean unimplemented = false;
- for (JField f : mFields) {
- if ((f.getType() instanceof JMap)
- || (f.getType() instanceof JVector))
- {
- unimplemented = true;
+ cs.write(" public void Deserialize(IInputArchive a_, String tag) {\n");
+ cs.write(" a_.StartRecord(tag);\n");
+ fIdx = 0;
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ cs.write(jf.genCsharpReadMethodName());
}
- }
- if (unimplemented) {
- cs.write(" throw new InvalidOperationException(\"comparing "
- + getCsharpName() + " is unimplemented\");\n");
- } else {
- cs.write(" if (!(peer_ is "+getCsharpName()+")) {\n");
- cs.write(" throw new InvalidOperationException(\"Comparing different types of records.\");\n");
+ cs.write(" a_.EndRecord(tag);\n");
+ cs.write("}\n");
+
+ cs.write(" public override String ToString() {\n");
+ cs.write(" try {\n");
+ cs.write(" System.IO.MemoryStream ms = new System.IO.MemoryStream();\n");
+ cs.write(" MiscUtil.IO.EndianBinaryWriter writer =\n");
+ cs.write(" new MiscUtil.IO.EndianBinaryWriter(MiscUtil.Conversion.EndianBitConverter.Big, ms, System.Text.Encoding.UTF8);\n");
+ cs.write(" BinaryOutputArchive a_ = \n");
+ cs.write(" new BinaryOutputArchive(writer);\n");
+ cs.write(" a_.StartRecord(this,\"\");\n");
+ fIdx = 0;
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ cs.write(jf.genCsharpWriteMethodName());
+ }
+ cs.write(" a_.EndRecord(this,\"\");\n");
+ cs.write(" ms.Position = 0;\n");
+ cs.write(" return System.Text.Encoding.UTF8.GetString(ms.ToArray());\n");
+ cs.write(" } catch (Exception ex) {\n");
+ cs.write(" Console.WriteLine(ex.StackTrace);\n");
cs.write(" }\n");
- cs.write(" "+getCsharpName()+" peer = ("+getCsharpName()+") peer_;\n");
- cs.write(" int ret = 0;\n");
+ cs.write(" return \"ERROR\";\n");
+ cs.write(" }\n");
+
+ cs.write(" public void Write(MiscUtil.IO.EndianBinaryWriter writer) {\n");
+ cs.write(" BinaryOutputArchive archive = new BinaryOutputArchive(writer);\n");
+ cs.write(" Serialize(archive, \"\");\n");
+ cs.write(" }\n");
+
+ cs.write(" public void ReadFields(MiscUtil.IO.EndianBinaryReader reader) {\n");
+ cs.write(" BinaryInputArchive archive = new BinaryInputArchive(reader);\n");
+ cs.write(" Deserialize(archive, \"\");\n");
+ cs.write(" }\n");
+
+ cs.write(" public int CompareTo (object peer_) {\n");
+ boolean unimplemented = false;
+ for (JField f : mFields) {
+ if ((f.getType() instanceof JMap)
+ || (f.getType() instanceof JVector)) {
+ unimplemented = true;
+ }
+ }
+ if (unimplemented) {
+ cs.write(" throw new InvalidOperationException(\"comparing "
+ + getCsharpName() + " is unimplemented\");\n");
+ } else {
+ cs.write(" if (!(peer_ is " + getCsharpName() + ")) {\n");
+ cs.write(" throw new InvalidOperationException(\"Comparing different types of records.\");\n");
+ cs.write(" }\n");
+ cs.write(" " + getCsharpName() + " peer = (" + getCsharpName() + ") peer_;\n");
+ cs.write(" int ret = 0;\n");
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ cs.write(jf.genCsharpCompareTo());
+ cs.write(" if (ret != 0) return ret;\n");
+ }
+ cs.write(" return ret;\n");
+ }
+ cs.write(" }\n");
+
+ cs.write(" public override bool Equals(object peer_) {\n");
+ cs.write(" if (!(peer_ is " + getCsharpName() + ")) {\n");
+ cs.write(" return false;\n");
+ cs.write(" }\n");
+ cs.write(" if (peer_ == this) {\n");
+ cs.write(" return true;\n");
+ cs.write(" }\n");
+ cs.write(" bool ret = false;\n");
+ cs.write(" " + getCsharpName() + " peer = (" + getCsharpName() + ")peer_;\n");
for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
JField jf = i.next();
- cs.write(jf.genCsharpCompareTo());
- cs.write(" if (ret != 0) return ret;\n");
+ cs.write(jf.genCsharpEquals());
--- End diff --
What's the reason for changing to `genCsharpEquals`? I suppose is is the same reason for changing to `genJavaEquals`...
---
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] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by fpj <gi...@git.apache.org>.
Github user fpj commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/102#discussion_r89283699
--- Diff: src/java/main/org/apache/zookeeper/cli/DeleteCommand.java ---
@@ -56,14 +56,7 @@ public CliCommand parse(String[] cmdArgs) throws CliParseException {
}
private void retainCompatibility(String[] cmdArgs) throws CliParseException {
- // delete path [version]
if (args.length > 2) {
- // rewrite to option
- String [] newCmd = new String[4];
--- End diff --
Why are we removing this rewrite?
---
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] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by fpj <gi...@git.apache.org>.
Github user fpj commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/102#discussion_r89284431
--- Diff: src/java/test/config/findbugsExcludeFile.xml ---
@@ -144,4 +144,10 @@
<Bug pattern="DM_DEFAULT_ENCODING" />
</Match>
+ <!-- Disable 'Malicious code vulnerability warnings' due to mutable collection types in interface.
--- End diff --
We probably want to leave a note in ZOOKEEPER-1362 to remind ourselves to do 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] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by lvfangmin <gi...@git.apache.org>.
Github user lvfangmin commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/102#discussion_r87264725
--- Diff: src/java/main/org/apache/zookeeper/server/DataTree.java ---
@@ -1293,13 +1293,12 @@ public synchronized WatchesSummary getWatchesSummary() {
* @param pwriter the output to write to
*/
public void dumpEphemerals(PrintWriter pwriter) {
- Set<Long> keys = ephemerals.keySet();
pwriter.println("Sessions with Ephemerals ("
- + keys.size() + "):");
- for (long k : keys) {
- pwriter.print("0x" + Long.toHexString(k));
+ + ephemerals.keySet().size() + "):");
+ for (Entry<Long, HashSet<String>> entry : ephemerals.entrySet()) {
+ pwriter.print("0x" + Long.toHexString(entry.getKey()));
pwriter.println(":");
- HashSet<String> tmp = ephemerals.get(k);
+ HashSet<String> tmp = entry.getValue();
--- End diff --
Usually, prefer to use the interface if we only use the methods defined in the interface, which makes it flexible (or less change needed) in case we want to use another Set implementation.
There is a best practice defined in Effective Java 2nd Edition, Item 52: Refer to objects by their interfaces:
If appropriate interface types exist, then parameters, return values, and fields should all be declared using interface types. If you get into the habit of using interface types, your program will be much more flexible. It is entirely appropriate to refer to an object by a class if no appropriate interface exists.
---
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] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by breed <gi...@git.apache.org>.
Github user breed commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/102#discussion_r86902396
--- Diff: src/java/main/org/apache/jute/compiler/CGenerator.java ---
@@ -61,70 +61,88 @@ void genCode() throws IOException {
+ outputDirectory);
}
}
- FileWriter c = new FileWriter(new File(outputDirectory, mName+".c"));
- FileWriter h = new FileWriter(new File(outputDirectory, mName+".h"));
- h.write("/**\n");
- h.write("* Licensed to the Apache Software Foundation (ASF) under one\n");
- h.write("* or more contributor license agreements. See the NOTICE file\n");
- h.write("* distributed with this work for additional information\n");
- h.write("* regarding copyright ownership. The ASF licenses this file\n");
- h.write("* to you under the Apache License, Version 2.0 (the\n");
- h.write("* \"License\"); you may not use this file except in compliance\n");
- h.write("* with the License. You may obtain a copy of the License at\n");
- h.write("*\n");
- h.write("* http://www.apache.org/licenses/LICENSE-2.0\n");
- h.write("*\n");
- h.write("* Unless required by applicable law or agreed to in writing, software\n");
- h.write("* distributed under the License is distributed on an \"AS IS\" BASIS,\n");
- h.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n");
- h.write("* See the License for the specific language governing permissions and\n");
- h.write("* limitations under the License.\n");
- h.write("*/\n");
- h.write("\n");
+ FileWriter c = null, h = null;
+ try {
+ c = new FileWriter(new File(outputDirectory, mName + ".c"));
+ h = new FileWriter(new File(outputDirectory, mName + ".h"));
- c.write("/**\n");
- c.write("* Licensed to the Apache Software Foundation (ASF) under one\n");
- c.write("* or more contributor license agreements. See the NOTICE file\n");
- c.write("* distributed with this work for additional information\n");
- c.write("* regarding copyright ownership. The ASF licenses this file\n");
- c.write("* to you under the Apache License, Version 2.0 (the\n");
- c.write("* \"License\"); you may not use this file except in compliance\n");
- c.write("* with the License. You may obtain a copy of the License at\n");
- c.write("*\n");
- c.write("* http://www.apache.org/licenses/LICENSE-2.0\n");
- c.write("*\n");
- c.write("* Unless required by applicable law or agreed to in writing, software\n");
- c.write("* distributed under the License is distributed on an \"AS IS\" BASIS,\n");
- c.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n");
- c.write("* See the License for the specific language governing permissions and\n");
- c.write("* limitations under the License.\n");
- c.write("*/\n");
- c.write("\n");
+ h.write("/**\n");
+ h.write("* Licensed to the Apache Software Foundation (ASF) under one\n");
+ h.write("* or more contributor license agreements. See the NOTICE file\n");
+ h.write("* distributed with this work for additional information\n");
+ h.write("* regarding copyright ownership. The ASF licenses this file\n");
+ h.write("* to you under the Apache License, Version 2.0 (the\n");
+ h.write("* \"License\"); you may not use this file except in compliance\n");
+ h.write("* with the License. You may obtain a copy of the License at\n");
+ h.write("*\n");
+ h.write("* http://www.apache.org/licenses/LICENSE-2.0\n");
+ h.write("*\n");
+ h.write("* Unless required by applicable law or agreed to in writing, software\n");
+ h.write("* distributed under the License is distributed on an \"AS IS\" BASIS,\n");
+ h.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n");
+ h.write("* See the License for the specific language governing permissions and\n");
+ h.write("* limitations under the License.\n");
+ h.write("*/\n");
+ h.write("\n");
- h.write("#ifndef __"+mName.toUpperCase().replace('.','_')+"__\n");
- h.write("#define __"+mName.toUpperCase().replace('.','_')+"__\n");
+ c.write("/**\n");
+ c.write("* Licensed to the Apache Software Foundation (ASF) under one\n");
+ c.write("* or more contributor license agreements. See the NOTICE file\n");
+ c.write("* distributed with this work for additional information\n");
+ c.write("* regarding copyright ownership. The ASF licenses this file\n");
+ c.write("* to you under the Apache License, Version 2.0 (the\n");
+ c.write("* \"License\"); you may not use this file except in compliance\n");
+ c.write("* with the License. You may obtain a copy of the License at\n");
+ c.write("*\n");
+ c.write("* http://www.apache.org/licenses/LICENSE-2.0\n");
+ c.write("*\n");
+ c.write("* Unless required by applicable law or agreed to in writing, software\n");
+ c.write("* distributed under the License is distributed on an \"AS IS\" BASIS,\n");
+ c.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n");
+ c.write("* See the License for the specific language governing permissions and\n");
+ c.write("* limitations under the License.\n");
+ c.write("*/\n");
+ c.write("\n");
- h.write("#include \"recordio.h\"\n");
- for (Iterator<JFile> i = mInclFiles.iterator(); i.hasNext();) {
- JFile f = i.next();
- h.write("#include \""+f.getName()+".h\"\n");
- }
- // required for compilation from C++
- h.write("\n#ifdef __cplusplus\nextern \"C\" {\n#endif\n\n");
+ h.write("#ifndef __" + mName.toUpperCase().replace('.', '_') + "__\n");
+ h.write("#define __" + mName.toUpperCase().replace('.', '_') + "__\n");
- c.write("#include <stdlib.h>\n"); // need it for calloc() & free()
- c.write("#include \""+mName+".h\"\n\n");
+ h.write("#include \"recordio.h\"\n");
+ for (Iterator<JFile> i = mInclFiles.iterator(); i.hasNext(); ) {
+ JFile f = i.next();
+ h.write("#include \"" + f.getName() + ".h\"\n");
+ }
+ // required for compilation from C++
+ h.write("\n#ifdef __cplusplus\nextern \"C\" {\n#endif\n\n");
- for (Iterator<JRecord> i = mRecList.iterator(); i.hasNext();) {
- JRecord jr = i.next();
- jr.genCCode(h, c);
- }
+ c.write("#include <stdlib.h>\n"); // need it for calloc() & free()
+ c.write("#include \"" + mName + ".h\"\n\n");
- h.write("\n#ifdef __cplusplus\n}\n#endif\n\n");
- h.write("#endif //"+mName.toUpperCase().replace('.','_')+"__\n");
+ for (Iterator<JRecord> i = mRecList.iterator(); i.hasNext(); ) {
+ JRecord jr = i.next();
+ jr.genCCode(h, c);
+ }
- h.close();
- c.close();
+ h.write("\n#ifdef __cplusplus\n}\n#endif\n\n");
+ h.write("#endif //" + mName.toUpperCase().replace('.', '_') + "__\n");
+ } catch (IOException e) {
+ throw e;
+ } finally {
+ if (h != null) {
+ try {
+ h.close();
+ } catch (IOException ex) {
+ throw ex;
+ } finally {
+ if (c != null) {
+ c.close();
+ }
+ }
+ }
+ if (c != null) {
--- End diff --
i think try with resources would clean this up. we are on java7 now right?
---
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] zookeeper issue #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by fpj <gi...@git.apache.org>.
Github user fpj commented on the issue:
https://github.com/apache/zookeeper/pull/102
@hanm I've left a few comments, but in general looks pretty good. I noticed that there are also unaddressed comments from @lvfangmin and @eribeiro . Could you take care of those so that we can check this in, please?
---
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] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by eribeiro <gi...@git.apache.org>.
Github user eribeiro commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/102#discussion_r89287398
--- Diff: src/java/test/config/findbugsExcludeFile.xml ---
@@ -144,4 +144,10 @@
<Bug pattern="DM_DEFAULT_ENCODING" />
</Match>
+ <!-- Disable 'Malicious code vulnerability warnings' due to mutable collection types in interface.
--- End diff --
Gonna say that! At least a TODO mark I guess...
---
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] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by eribeiro <gi...@git.apache.org>.
Github user eribeiro commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/102#discussion_r87028116
--- Diff: src/java/main/org/apache/zookeeper/server/DataTree.java ---
@@ -1293,13 +1293,12 @@ public synchronized WatchesSummary getWatchesSummary() {
* @param pwriter the output to write to
*/
public void dumpEphemerals(PrintWriter pwriter) {
- Set<Long> keys = ephemerals.keySet();
pwriter.println("Sessions with Ephemerals ("
- + keys.size() + "):");
- for (long k : keys) {
- pwriter.print("0x" + Long.toHexString(k));
+ + ephemerals.keySet().size() + "):");
+ for (Entry<Long, HashSet<String>> entry : ephemerals.entrySet()) {
+ pwriter.print("0x" + Long.toHexString(entry.getKey()));
pwriter.println(":");
- HashSet<String> tmp = ephemerals.get(k);
+ HashSet<String> tmp = entry.getValue();
--- End diff --
Could we define `tmp` as `Set` instead of `HashSet`?
---
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] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/zookeeper/pull/102
---
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] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by eribeiro <gi...@git.apache.org>.
Github user eribeiro commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/102#discussion_r87027302
--- Diff: src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumHierarchical.java ---
@@ -344,9 +345,10 @@ public boolean containsQuorum(Set<Long> set){
* Check if all groups have majority
*/
int majGroupCounter = 0;
- for(long gid : expansion.keySet()) {
- LOG.debug("Group info: " + expansion.get(gid) + ", " + gid + ", " + groupWeight.get(gid));
- if(expansion.get(gid) > (groupWeight.get(gid) / 2) )
+ for (Entry<Long, Long> entry : expansion.entrySet()) {
+ Long gid = entry.getKey();
+ LOG.debug("Group info: " + entry.getValue() + ", " + gid + ", " + groupWeight.get(gid));
+ if(entry.getValue() > (groupWeight.get(gid) / 2) )
--- End diff --
nit: add space after the `if` and remove space before the last parenthesis.
---
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] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by breed <gi...@git.apache.org>.
Github user breed commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/102#discussion_r86903124
--- Diff: src/java/main/org/apache/zookeeper/version/util/VerGen.java ---
@@ -93,6 +93,14 @@ public static void generateFile(File outputDir, Version version, int rev, String
} catch (IOException e) {
System.out.println("Unable to generate version.Info file: "
+ e.getMessage());
+ if (w != null) {
--- End diff --
wow this is a weird one... we have it in the finally... try with resources would make this a bit nicer too...
---
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] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by eribeiro <gi...@git.apache.org>.
Github user eribeiro commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/102#discussion_r87466839
--- Diff: src/java/main/org/apache/zookeeper/server/DataTree.java ---
@@ -1293,13 +1293,12 @@ public synchronized WatchesSummary getWatchesSummary() {
* @param pwriter the output to write to
*/
public void dumpEphemerals(PrintWriter pwriter) {
- Set<Long> keys = ephemerals.keySet();
pwriter.println("Sessions with Ephemerals ("
- + keys.size() + "):");
- for (long k : keys) {
- pwriter.print("0x" + Long.toHexString(k));
+ + ephemerals.keySet().size() + "):");
+ for (Entry<Long, HashSet<String>> entry : ephemerals.entrySet()) {
+ pwriter.print("0x" + Long.toHexString(entry.getKey()));
pwriter.println(":");
- HashSet<String> tmp = ephemerals.get(k);
+ HashSet<String> tmp = entry.getValue();
--- End diff --
Ya @lvfangmin, one of my favourite books on Java, and was the reference to my comment above. Thanks pointing out. :smiley:
Agree with @hanm, better to create a separate issue for dealing with this legacy technical debt. :+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] zookeeper issue #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on the issue:
https://github.com/apache/zookeeper/pull/102
Updated PR to address review comments from @breed.
---
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] zookeeper issue #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on the issue:
https://github.com/apache/zookeeper/pull/102
Updated PR to address review comments from @eribeiro.
---
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] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by fpj <gi...@git.apache.org>.
Github user fpj commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/102#discussion_r89264584
--- Diff: src/java/main/org/apache/jute/compiler/JRecord.java ---
@@ -403,168 +410,165 @@ public void genJavaCode(File outputDirectory) throws IOException {
} else if (!pkgdir.isDirectory()) {
throw new IOException(pkgpath + " is not a directory.");
}
- File jfile = new File(pkgdir, getName()+".java");
- FileWriter jj = new FileWriter(jfile);
- jj.write("// File generated by hadoop record compiler. Do not edit.\n");
- jj.write("/**\n");
- jj.write("* Licensed to the Apache Software Foundation (ASF) under one\n");
- jj.write("* or more contributor license agreements. See the NOTICE file\n");
- jj.write("* distributed with this work for additional information\n");
- jj.write("* regarding copyright ownership. The ASF licenses this file\n");
- jj.write("* to you under the Apache License, Version 2.0 (the\n");
- jj.write("* \"License\"); you may not use this file except in compliance\n");
- jj.write("* with the License. You may obtain a copy of the License at\n");
- jj.write("*\n");
- jj.write("* http://www.apache.org/licenses/LICENSE-2.0\n");
- jj.write("*\n");
- jj.write("* Unless required by applicable law or agreed to in writing, software\n");
- jj.write("* distributed under the License is distributed on an \"AS IS\" BASIS,\n");
- jj.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n");
- jj.write("* See the License for the specific language governing permissions and\n");
- jj.write("* limitations under the License.\n");
- jj.write("*/\n");
- jj.write("\n");
- jj.write("package "+getJavaPackage()+";\n\n");
- jj.write("import org.apache.jute.*;\n");
- jj.write("public class "+getName()+" implements Record {\n");
- for (Iterator<JField> i = mFields.iterator(); i.hasNext();) {
- JField jf = i.next();
- jj.write(jf.genJavaDecl());
- }
- jj.write(" public "+getName()+"() {\n");
- jj.write(" }\n");
+ try (FileWriter jj = new FileWriter(new File(pkgdir, getName()+".java"))) {
+ jj.write("// File generated by hadoop record compiler. Do not edit.\n");
+ jj.write("/**\n");
+ jj.write("* Licensed to the Apache Software Foundation (ASF) under one\n");
+ jj.write("* or more contributor license agreements. See the NOTICE file\n");
+ jj.write("* distributed with this work for additional information\n");
+ jj.write("* regarding copyright ownership. The ASF licenses this file\n");
+ jj.write("* to you under the Apache License, Version 2.0 (the\n");
+ jj.write("* \"License\"); you may not use this file except in compliance\n");
+ jj.write("* with the License. You may obtain a copy of the License at\n");
+ jj.write("*\n");
+ jj.write("* http://www.apache.org/licenses/LICENSE-2.0\n");
+ jj.write("*\n");
+ jj.write("* Unless required by applicable law or agreed to in writing, software\n");
+ jj.write("* distributed under the License is distributed on an \"AS IS\" BASIS,\n");
+ jj.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n");
+ jj.write("* See the License for the specific language governing permissions and\n");
+ jj.write("* limitations under the License.\n");
+ jj.write("*/\n");
+ jj.write("\n");
+ jj.write("package " + getJavaPackage() + ";\n\n");
+ jj.write("import org.apache.jute.*;\n");
+ jj.write("public class " + getName() + " implements Record {\n");
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); ) {
+ JField jf = i.next();
+ jj.write(jf.genJavaDecl());
+ }
+ jj.write(" public " + getName() + "() {\n");
+ jj.write(" }\n");
- jj.write(" public "+getName()+"(\n");
- int fIdx = 0;
- int fLen = mFields.size();
- for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
- JField jf = i.next();
- jj.write(jf.genJavaConstructorParam(jf.getName()));
- jj.write((fLen-1 == fIdx)?"":",\n");
- }
- jj.write(") {\n");
- fIdx = 0;
- for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
- JField jf = i.next();
- jj.write(jf.genJavaConstructorSet(jf.getName()));
- }
- jj.write(" }\n");
- fIdx = 0;
- for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
- JField jf = i.next();
- jj.write(jf.genJavaGetSet(fIdx));
- }
- jj.write(" public void serialize(OutputArchive a_, String tag) throws java.io.IOException {\n");
- jj.write(" a_.startRecord(this,tag);\n");
- fIdx = 0;
- for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
- JField jf = i.next();
- jj.write(jf.genJavaWriteMethodName());
- }
- jj.write(" a_.endRecord(this,tag);\n");
- jj.write(" }\n");
+ jj.write(" public " + getName() + "(\n");
+ int fIdx = 0;
+ int fLen = mFields.size();
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ jj.write(jf.genJavaConstructorParam(jf.getName()));
+ jj.write((fLen - 1 == fIdx) ? "" : ",\n");
+ }
+ jj.write(") {\n");
+ fIdx = 0;
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ jj.write(jf.genJavaConstructorSet(jf.getName()));
+ }
+ jj.write(" }\n");
+ fIdx = 0;
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ jj.write(jf.genJavaGetSet(fIdx));
+ }
+ jj.write(" public void serialize(OutputArchive a_, String tag) throws java.io.IOException {\n");
+ jj.write(" a_.startRecord(this,tag);\n");
+ fIdx = 0;
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ jj.write(jf.genJavaWriteMethodName());
+ }
+ jj.write(" a_.endRecord(this,tag);\n");
+ jj.write(" }\n");
- jj.write(" public void deserialize(InputArchive a_, String tag) throws java.io.IOException {\n");
- jj.write(" a_.startRecord(tag);\n");
- fIdx = 0;
- for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
- JField jf = i.next();
- jj.write(jf.genJavaReadMethodName());
- }
- jj.write(" a_.endRecord(tag);\n");
- jj.write("}\n");
-
- jj.write(" public String toString() {\n");
- jj.write(" try {\n");
- jj.write(" java.io.ByteArrayOutputStream s =\n");
- jj.write(" new java.io.ByteArrayOutputStream();\n");
- jj.write(" CsvOutputArchive a_ = \n");
- jj.write(" new CsvOutputArchive(s);\n");
- jj.write(" a_.startRecord(this,\"\");\n");
- fIdx = 0;
- for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
- JField jf = i.next();
- jj.write(jf.genJavaWriteMethodName());
- }
- jj.write(" a_.endRecord(this,\"\");\n");
- jj.write(" return new String(s.toByteArray(), \"UTF-8\");\n");
- jj.write(" } catch (Throwable ex) {\n");
- jj.write(" ex.printStackTrace();\n");
- jj.write(" }\n");
- jj.write(" return \"ERROR\";\n");
- jj.write(" }\n");
-
- jj.write(" public void write(java.io.DataOutput out) throws java.io.IOException {\n");
- jj.write(" BinaryOutputArchive archive = new BinaryOutputArchive(out);\n");
- jj.write(" serialize(archive, \"\");\n");
- jj.write(" }\n");
-
- jj.write(" public void readFields(java.io.DataInput in) throws java.io.IOException {\n");
- jj.write(" BinaryInputArchive archive = new BinaryInputArchive(in);\n");
- jj.write(" deserialize(archive, \"\");\n");
- jj.write(" }\n");
-
- jj.write(" public int compareTo (Object peer_) throws ClassCastException {\n");
- boolean unimplemented = false;
- for (JField f : mFields) {
- if ((f.getType() instanceof JMap)
- || (f.getType() instanceof JVector))
- {
- unimplemented = true;
+ jj.write(" public void deserialize(InputArchive a_, String tag) throws java.io.IOException {\n");
+ jj.write(" a_.startRecord(tag);\n");
+ fIdx = 0;
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ jj.write(jf.genJavaReadMethodName());
}
- }
- if (unimplemented) {
- jj.write(" throw new UnsupportedOperationException(\"comparing "
- + getName() + " is unimplemented\");\n");
- } else {
- jj.write(" if (!(peer_ instanceof "+getName()+")) {\n");
- jj.write(" throw new ClassCastException(\"Comparing different types of records.\");\n");
+ jj.write(" a_.endRecord(tag);\n");
+ jj.write("}\n");
+
+ jj.write(" public String toString() {\n");
+ jj.write(" try {\n");
+ jj.write(" java.io.ByteArrayOutputStream s =\n");
+ jj.write(" new java.io.ByteArrayOutputStream();\n");
+ jj.write(" CsvOutputArchive a_ = \n");
+ jj.write(" new CsvOutputArchive(s);\n");
+ jj.write(" a_.startRecord(this,\"\");\n");
+ fIdx = 0;
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ jj.write(jf.genJavaWriteMethodName());
+ }
+ jj.write(" a_.endRecord(this,\"\");\n");
+ jj.write(" return new String(s.toByteArray(), \"UTF-8\");\n");
+ jj.write(" } catch (Throwable ex) {\n");
+ jj.write(" ex.printStackTrace();\n");
jj.write(" }\n");
- jj.write(" "+getName()+" peer = ("+getName()+") peer_;\n");
- jj.write(" int ret = 0;\n");
+ jj.write(" return \"ERROR\";\n");
+ jj.write(" }\n");
+
+ jj.write(" public void write(java.io.DataOutput out) throws java.io.IOException {\n");
+ jj.write(" BinaryOutputArchive archive = new BinaryOutputArchive(out);\n");
+ jj.write(" serialize(archive, \"\");\n");
+ jj.write(" }\n");
+
+ jj.write(" public void readFields(java.io.DataInput in) throws java.io.IOException {\n");
+ jj.write(" BinaryInputArchive archive = new BinaryInputArchive(in);\n");
+ jj.write(" deserialize(archive, \"\");\n");
+ jj.write(" }\n");
+
+ jj.write(" public int compareTo (Object peer_) throws ClassCastException {\n");
+ boolean unimplemented = false;
+ for (JField f : mFields) {
+ if ((f.getType() instanceof JMap)
+ || (f.getType() instanceof JVector)) {
+ unimplemented = true;
+ }
+ }
+ if (unimplemented) {
+ jj.write(" throw new UnsupportedOperationException(\"comparing "
+ + getName() + " is unimplemented\");\n");
+ } else {
+ jj.write(" if (!(peer_ instanceof " + getName() + ")) {\n");
+ jj.write(" throw new ClassCastException(\"Comparing different types of records.\");\n");
+ jj.write(" }\n");
+ jj.write(" " + getName() + " peer = (" + getName() + ") peer_;\n");
+ jj.write(" int ret = 0;\n");
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ jj.write(jf.genJavaCompareTo());
+ jj.write(" if (ret != 0) return ret;\n");
+ }
+ jj.write(" return ret;\n");
+ }
+ jj.write(" }\n");
+
+ jj.write(" public boolean equals(Object peer_) {\n");
+ jj.write(" if (!(peer_ instanceof " + getName() + ")) {\n");
+ jj.write(" return false;\n");
+ jj.write(" }\n");
+ jj.write(" if (peer_ == this) {\n");
+ jj.write(" return true;\n");
+ jj.write(" }\n");
+ jj.write(" " + getName() + " peer = (" + getName() + ") peer_;\n");
+ jj.write(" boolean ret = false;\n");
for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
JField jf = i.next();
- jj.write(jf.genJavaCompareTo());
- jj.write(" if (ret != 0) return ret;\n");
+ jj.write(jf.genJavaEquals());
--- End diff --
What's the reason for changing to `genJavaEquals`?
---
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] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/102#discussion_r89356018
--- Diff: src/java/main/org/apache/jute/compiler/JRecord.java ---
@@ -576,174 +580,174 @@ public void genCsharpCode(File outputDirectory) throws IOException {
} else if (!outputDirectory.isDirectory()) {
throw new IOException(outputDirectory + " is not a directory.");
}
- File csharpFile = new File(outputDirectory, getName()+".cs");
- FileWriter cs = new FileWriter(csharpFile);
- cs.write("// File generated by hadoop record compiler. Do not edit.\n");
- cs.write("/**\n");
- cs.write("* Licensed to the Apache Software Foundation (ASF) under one\n");
- cs.write("* or more contributor license agreements. See the NOTICE file\n");
- cs.write("* distributed with this work for additional information\n");
- cs.write("* regarding copyright ownership. The ASF licenses this file\n");
- cs.write("* to you under the Apache License, Version 2.0 (the\n");
- cs.write("* \"License\"); you may not use this file except in compliance\n");
- cs.write("* with the License. You may obtain a copy of the License at\n");
- cs.write("*\n");
- cs.write("* http://www.apache.org/licenses/LICENSE-2.0\n");
- cs.write("*\n");
- cs.write("* Unless required by applicable law or agreed to in writing, software\n");
- cs.write("* distributed under the License is distributed on an \"AS IS\" BASIS,\n");
- cs.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n");
- cs.write("* See the License for the specific language governing permissions and\n");
- cs.write("* limitations under the License.\n");
- cs.write("*/\n");
- cs.write("\n");
- cs.write("using System;\n");
- cs.write("using Org.Apache.Jute;\n");
- cs.write("\n");
- cs.write("namespace "+getCsharpNameSpace()+"\n");
- cs.write("{\n");
-
- String className = getCsharpName();
- cs.write("public class "+className+" : IRecord, IComparable \n");
- cs.write("{\n");
- cs.write(" public "+ className +"() {\n");
- cs.write(" }\n");
-
- cs.write(" public "+className+"(\n");
- int fIdx = 0;
- int fLen = mFields.size();
- for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
- JField jf = i.next();
- cs.write(jf.genCsharpConstructorParam(jf.getCsharpName()));
- cs.write((fLen-1 == fIdx)?"":",\n");
- }
- cs.write(") {\n");
- fIdx = 0;
- for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
- JField jf = i.next();
- cs.write(jf.genCsharpConstructorSet(jf.getCsharpName()));
- }
- cs.write(" }\n");
- fIdx = 0;
- for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
- JField jf = i.next();
- cs.write(jf.genCsharpGetSet(fIdx));
+
+ try (FileWriter cs = new FileWriter(new File(outputDirectory, getName() + ".cs"));) {
+ cs.write("// File generated by hadoop record compiler. Do not edit.\n");
+ cs.write("/**\n");
+ cs.write("* Licensed to the Apache Software Foundation (ASF) under one\n");
+ cs.write("* or more contributor license agreements. See the NOTICE file\n");
+ cs.write("* distributed with this work for additional information\n");
+ cs.write("* regarding copyright ownership. The ASF licenses this file\n");
+ cs.write("* to you under the Apache License, Version 2.0 (the\n");
+ cs.write("* \"License\"); you may not use this file except in compliance\n");
+ cs.write("* with the License. You may obtain a copy of the License at\n");
+ cs.write("*\n");
+ cs.write("* http://www.apache.org/licenses/LICENSE-2.0\n");
+ cs.write("*\n");
+ cs.write("* Unless required by applicable law or agreed to in writing, software\n");
+ cs.write("* distributed under the License is distributed on an \"AS IS\" BASIS,\n");
+ cs.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n");
+ cs.write("* See the License for the specific language governing permissions and\n");
+ cs.write("* limitations under the License.\n");
+ cs.write("*/\n");
cs.write("\n");
- }
- cs.write(" public void Serialize(IOutputArchive a_, String tag) {\n");
- cs.write(" a_.StartRecord(this,tag);\n");
- fIdx = 0;
- for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
- JField jf = i.next();
- cs.write(jf.genCsharpWriteMethodName());
- }
- cs.write(" a_.EndRecord(this,tag);\n");
- cs.write(" }\n");
+ cs.write("using System;\n");
+ cs.write("using Org.Apache.Jute;\n");
+ cs.write("\n");
+ cs.write("namespace " + getCsharpNameSpace() + "\n");
+ cs.write("{\n");
+
+ String className = getCsharpName();
+ cs.write("public class " + className + " : IRecord, IComparable \n");
+ cs.write("{\n");
+ cs.write(" public " + className + "() {\n");
+ cs.write(" }\n");
+
+ cs.write(" public " + className + "(\n");
+ int fIdx = 0;
+ int fLen = mFields.size();
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ cs.write(jf.genCsharpConstructorParam(jf.getCsharpName()));
+ cs.write((fLen - 1 == fIdx) ? "" : ",\n");
+ }
+ cs.write(") {\n");
+ fIdx = 0;
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ cs.write(jf.genCsharpConstructorSet(jf.getCsharpName()));
+ }
+ cs.write(" }\n");
+ fIdx = 0;
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ cs.write(jf.genCsharpGetSet(fIdx));
+ cs.write("\n");
+ }
+ cs.write(" public void Serialize(IOutputArchive a_, String tag) {\n");
+ cs.write(" a_.StartRecord(this,tag);\n");
+ fIdx = 0;
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ cs.write(jf.genCsharpWriteMethodName());
+ }
+ cs.write(" a_.EndRecord(this,tag);\n");
+ cs.write(" }\n");
- cs.write(" public void Deserialize(IInputArchive a_, String tag) {\n");
- cs.write(" a_.StartRecord(tag);\n");
- fIdx = 0;
- for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
- JField jf = i.next();
- cs.write(jf.genCsharpReadMethodName());
- }
- cs.write(" a_.EndRecord(tag);\n");
- cs.write("}\n");
-
- cs.write(" public override String ToString() {\n");
- cs.write(" try {\n");
- cs.write(" System.IO.MemoryStream ms = new System.IO.MemoryStream();\n");
- cs.write(" MiscUtil.IO.EndianBinaryWriter writer =\n");
- cs.write(" new MiscUtil.IO.EndianBinaryWriter(MiscUtil.Conversion.EndianBitConverter.Big, ms, System.Text.Encoding.UTF8);\n");
- cs.write(" BinaryOutputArchive a_ = \n");
- cs.write(" new BinaryOutputArchive(writer);\n");
- cs.write(" a_.StartRecord(this,\"\");\n");
- fIdx = 0;
- for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
- JField jf = i.next();
- cs.write(jf.genCsharpWriteMethodName());
- }
- cs.write(" a_.EndRecord(this,\"\");\n");
- cs.write(" ms.Position = 0;\n");
- cs.write(" return System.Text.Encoding.UTF8.GetString(ms.ToArray());\n");
- cs.write(" } catch (Exception ex) {\n");
- cs.write(" Console.WriteLine(ex.StackTrace);\n");
- cs.write(" }\n");
- cs.write(" return \"ERROR\";\n");
- cs.write(" }\n");
-
- cs.write(" public void Write(MiscUtil.IO.EndianBinaryWriter writer) {\n");
- cs.write(" BinaryOutputArchive archive = new BinaryOutputArchive(writer);\n");
- cs.write(" Serialize(archive, \"\");\n");
- cs.write(" }\n");
-
- cs.write(" public void ReadFields(MiscUtil.IO.EndianBinaryReader reader) {\n");
- cs.write(" BinaryInputArchive archive = new BinaryInputArchive(reader);\n");
- cs.write(" Deserialize(archive, \"\");\n");
- cs.write(" }\n");
-
- cs.write(" public int CompareTo (object peer_) {\n");
- boolean unimplemented = false;
- for (JField f : mFields) {
- if ((f.getType() instanceof JMap)
- || (f.getType() instanceof JVector))
- {
- unimplemented = true;
+ cs.write(" public void Deserialize(IInputArchive a_, String tag) {\n");
+ cs.write(" a_.StartRecord(tag);\n");
+ fIdx = 0;
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ cs.write(jf.genCsharpReadMethodName());
}
- }
- if (unimplemented) {
- cs.write(" throw new InvalidOperationException(\"comparing "
- + getCsharpName() + " is unimplemented\");\n");
- } else {
- cs.write(" if (!(peer_ is "+getCsharpName()+")) {\n");
- cs.write(" throw new InvalidOperationException(\"Comparing different types of records.\");\n");
+ cs.write(" a_.EndRecord(tag);\n");
+ cs.write("}\n");
+
+ cs.write(" public override String ToString() {\n");
+ cs.write(" try {\n");
+ cs.write(" System.IO.MemoryStream ms = new System.IO.MemoryStream();\n");
+ cs.write(" MiscUtil.IO.EndianBinaryWriter writer =\n");
+ cs.write(" new MiscUtil.IO.EndianBinaryWriter(MiscUtil.Conversion.EndianBitConverter.Big, ms, System.Text.Encoding.UTF8);\n");
+ cs.write(" BinaryOutputArchive a_ = \n");
+ cs.write(" new BinaryOutputArchive(writer);\n");
+ cs.write(" a_.StartRecord(this,\"\");\n");
+ fIdx = 0;
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ cs.write(jf.genCsharpWriteMethodName());
+ }
+ cs.write(" a_.EndRecord(this,\"\");\n");
+ cs.write(" ms.Position = 0;\n");
+ cs.write(" return System.Text.Encoding.UTF8.GetString(ms.ToArray());\n");
+ cs.write(" } catch (Exception ex) {\n");
+ cs.write(" Console.WriteLine(ex.StackTrace);\n");
cs.write(" }\n");
- cs.write(" "+getCsharpName()+" peer = ("+getCsharpName()+") peer_;\n");
- cs.write(" int ret = 0;\n");
+ cs.write(" return \"ERROR\";\n");
+ cs.write(" }\n");
+
+ cs.write(" public void Write(MiscUtil.IO.EndianBinaryWriter writer) {\n");
+ cs.write(" BinaryOutputArchive archive = new BinaryOutputArchive(writer);\n");
+ cs.write(" Serialize(archive, \"\");\n");
+ cs.write(" }\n");
+
+ cs.write(" public void ReadFields(MiscUtil.IO.EndianBinaryReader reader) {\n");
+ cs.write(" BinaryInputArchive archive = new BinaryInputArchive(reader);\n");
+ cs.write(" Deserialize(archive, \"\");\n");
+ cs.write(" }\n");
+
+ cs.write(" public int CompareTo (object peer_) {\n");
+ boolean unimplemented = false;
+ for (JField f : mFields) {
+ if ((f.getType() instanceof JMap)
+ || (f.getType() instanceof JVector)) {
+ unimplemented = true;
+ }
+ }
+ if (unimplemented) {
+ cs.write(" throw new InvalidOperationException(\"comparing "
+ + getCsharpName() + " is unimplemented\");\n");
+ } else {
+ cs.write(" if (!(peer_ is " + getCsharpName() + ")) {\n");
+ cs.write(" throw new InvalidOperationException(\"Comparing different types of records.\");\n");
+ cs.write(" }\n");
+ cs.write(" " + getCsharpName() + " peer = (" + getCsharpName() + ") peer_;\n");
+ cs.write(" int ret = 0;\n");
+ for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
+ JField jf = i.next();
+ cs.write(jf.genCsharpCompareTo());
+ cs.write(" if (ret != 0) return ret;\n");
+ }
+ cs.write(" return ret;\n");
+ }
+ cs.write(" }\n");
+
+ cs.write(" public override bool Equals(object peer_) {\n");
+ cs.write(" if (!(peer_ is " + getCsharpName() + ")) {\n");
+ cs.write(" return false;\n");
+ cs.write(" }\n");
+ cs.write(" if (peer_ == this) {\n");
+ cs.write(" return true;\n");
+ cs.write(" }\n");
+ cs.write(" bool ret = false;\n");
+ cs.write(" " + getCsharpName() + " peer = (" + getCsharpName() + ")peer_;\n");
for (Iterator<JField> i = mFields.iterator(); i.hasNext(); fIdx++) {
JField jf = i.next();
- cs.write(jf.genCsharpCompareTo());
- cs.write(" if (ret != 0) return ret;\n");
+ cs.write(jf.genCsharpEquals());
--- End diff --
Similarly, there is no actual change here.
---
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] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by hanm <gi...@git.apache.org>.
Github user hanm closed the pull request at:
https://github.com/apache/zookeeper/pull/102
---
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] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/102#discussion_r86905102
--- Diff: src/java/main/org/apache/jute/compiler/CGenerator.java ---
@@ -61,70 +61,88 @@ void genCode() throws IOException {
+ outputDirectory);
}
}
- FileWriter c = new FileWriter(new File(outputDirectory, mName+".c"));
- FileWriter h = new FileWriter(new File(outputDirectory, mName+".h"));
- h.write("/**\n");
- h.write("* Licensed to the Apache Software Foundation (ASF) under one\n");
- h.write("* or more contributor license agreements. See the NOTICE file\n");
- h.write("* distributed with this work for additional information\n");
- h.write("* regarding copyright ownership. The ASF licenses this file\n");
- h.write("* to you under the Apache License, Version 2.0 (the\n");
- h.write("* \"License\"); you may not use this file except in compliance\n");
- h.write("* with the License. You may obtain a copy of the License at\n");
- h.write("*\n");
- h.write("* http://www.apache.org/licenses/LICENSE-2.0\n");
- h.write("*\n");
- h.write("* Unless required by applicable law or agreed to in writing, software\n");
- h.write("* distributed under the License is distributed on an \"AS IS\" BASIS,\n");
- h.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n");
- h.write("* See the License for the specific language governing permissions and\n");
- h.write("* limitations under the License.\n");
- h.write("*/\n");
- h.write("\n");
+ FileWriter c = null, h = null;
+ try {
+ c = new FileWriter(new File(outputDirectory, mName + ".c"));
+ h = new FileWriter(new File(outputDirectory, mName + ".h"));
- c.write("/**\n");
- c.write("* Licensed to the Apache Software Foundation (ASF) under one\n");
- c.write("* or more contributor license agreements. See the NOTICE file\n");
- c.write("* distributed with this work for additional information\n");
- c.write("* regarding copyright ownership. The ASF licenses this file\n");
- c.write("* to you under the Apache License, Version 2.0 (the\n");
- c.write("* \"License\"); you may not use this file except in compliance\n");
- c.write("* with the License. You may obtain a copy of the License at\n");
- c.write("*\n");
- c.write("* http://www.apache.org/licenses/LICENSE-2.0\n");
- c.write("*\n");
- c.write("* Unless required by applicable law or agreed to in writing, software\n");
- c.write("* distributed under the License is distributed on an \"AS IS\" BASIS,\n");
- c.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n");
- c.write("* See the License for the specific language governing permissions and\n");
- c.write("* limitations under the License.\n");
- c.write("*/\n");
- c.write("\n");
+ h.write("/**\n");
+ h.write("* Licensed to the Apache Software Foundation (ASF) under one\n");
+ h.write("* or more contributor license agreements. See the NOTICE file\n");
+ h.write("* distributed with this work for additional information\n");
+ h.write("* regarding copyright ownership. The ASF licenses this file\n");
+ h.write("* to you under the Apache License, Version 2.0 (the\n");
+ h.write("* \"License\"); you may not use this file except in compliance\n");
+ h.write("* with the License. You may obtain a copy of the License at\n");
+ h.write("*\n");
+ h.write("* http://www.apache.org/licenses/LICENSE-2.0\n");
+ h.write("*\n");
+ h.write("* Unless required by applicable law or agreed to in writing, software\n");
+ h.write("* distributed under the License is distributed on an \"AS IS\" BASIS,\n");
+ h.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n");
+ h.write("* See the License for the specific language governing permissions and\n");
+ h.write("* limitations under the License.\n");
+ h.write("*/\n");
+ h.write("\n");
- h.write("#ifndef __"+mName.toUpperCase().replace('.','_')+"__\n");
- h.write("#define __"+mName.toUpperCase().replace('.','_')+"__\n");
+ c.write("/**\n");
+ c.write("* Licensed to the Apache Software Foundation (ASF) under one\n");
+ c.write("* or more contributor license agreements. See the NOTICE file\n");
+ c.write("* distributed with this work for additional information\n");
+ c.write("* regarding copyright ownership. The ASF licenses this file\n");
+ c.write("* to you under the Apache License, Version 2.0 (the\n");
+ c.write("* \"License\"); you may not use this file except in compliance\n");
+ c.write("* with the License. You may obtain a copy of the License at\n");
+ c.write("*\n");
+ c.write("* http://www.apache.org/licenses/LICENSE-2.0\n");
+ c.write("*\n");
+ c.write("* Unless required by applicable law or agreed to in writing, software\n");
+ c.write("* distributed under the License is distributed on an \"AS IS\" BASIS,\n");
+ c.write("* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n");
+ c.write("* See the License for the specific language governing permissions and\n");
+ c.write("* limitations under the License.\n");
+ c.write("*/\n");
+ c.write("\n");
- h.write("#include \"recordio.h\"\n");
- for (Iterator<JFile> i = mInclFiles.iterator(); i.hasNext();) {
- JFile f = i.next();
- h.write("#include \""+f.getName()+".h\"\n");
- }
- // required for compilation from C++
- h.write("\n#ifdef __cplusplus\nextern \"C\" {\n#endif\n\n");
+ h.write("#ifndef __" + mName.toUpperCase().replace('.', '_') + "__\n");
+ h.write("#define __" + mName.toUpperCase().replace('.', '_') + "__\n");
- c.write("#include <stdlib.h>\n"); // need it for calloc() & free()
- c.write("#include \""+mName+".h\"\n\n");
+ h.write("#include \"recordio.h\"\n");
+ for (Iterator<JFile> i = mInclFiles.iterator(); i.hasNext(); ) {
+ JFile f = i.next();
+ h.write("#include \"" + f.getName() + ".h\"\n");
+ }
+ // required for compilation from C++
+ h.write("\n#ifdef __cplusplus\nextern \"C\" {\n#endif\n\n");
- for (Iterator<JRecord> i = mRecList.iterator(); i.hasNext();) {
- JRecord jr = i.next();
- jr.genCCode(h, c);
- }
+ c.write("#include <stdlib.h>\n"); // need it for calloc() & free()
+ c.write("#include \"" + mName + ".h\"\n\n");
- h.write("\n#ifdef __cplusplus\n}\n#endif\n\n");
- h.write("#endif //"+mName.toUpperCase().replace('.','_')+"__\n");
+ for (Iterator<JRecord> i = mRecList.iterator(); i.hasNext(); ) {
+ JRecord jr = i.next();
+ jr.genCCode(h, c);
+ }
- h.close();
- c.close();
+ h.write("\n#ifdef __cplusplus\n}\n#endif\n\n");
+ h.write("#endif //" + mName.toUpperCase().replace('.', '_') + "__\n");
+ } catch (IOException e) {
+ throw e;
+ } finally {
+ if (h != null) {
+ try {
+ h.close();
+ } catch (IOException ex) {
+ throw ex;
+ } finally {
+ if (c != null) {
+ c.close();
+ }
+ }
+ }
+ if (c != null) {
--- End diff --
Yeah I thought about using try / with - for some reasons I had the impression that we also need to support JDK 1.6 (that's probably only for 3.4.x). I'll update with try / with.
---
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] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by hanm <gi...@git.apache.org>.
GitHub user hanm reopened a pull request:
https://github.com/apache/zookeeper/pull/102
ZOOKEEPER-2628: Fix findbug warnings.
This PR fixed 19 find bug warnings except this one, which might require interface change that potentially could break client side apps.
Malicious code vulnerability Warnings
org.apache.zookeeper.ZooDefs$Ids.OPEN_ACL_UNSAFE is a mutable collection
Bug type MS_MUTABLE_COLLECTION (click for details)
In class org.apache.zookeeper.ZooDefs$Ids
Field org.apache.zookeeper.ZooDefs$Ids.OPEN_ACL_UNSAFE
At ZooDefs.java:[line 116]
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/hanm/zookeeper ZOOKEEPER-2628
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zookeeper/pull/102.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 #102
----
commit fbb0957167f42ba64ab3eab15fae4b8dce9b0f4b
Author: Michael Han <ha...@cloudera.com>
Date: 2016-11-15T23:09:02Z
ZOOKEEPER-2628: Fix findbug warnings.
----
---
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] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/102#discussion_r86905174
--- Diff: src/java/main/org/apache/zookeeper/version/util/VerGen.java ---
@@ -93,6 +93,14 @@ public static void generateFile(File outputDir, Version version, int rev, String
} catch (IOException e) {
System.out.println("Unable to generate version.Info file: "
+ e.getMessage());
+ if (w != null) {
--- End diff --
Findbug complains here because we use System.exit in catch block - in that case it is not guaranteed that the finally block will be executed.
---
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] zookeeper pull request #102: ZOOKEEPER-2628: Fix findbug warnings.
Posted by breed <gi...@git.apache.org>.
Github user breed commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/102#discussion_r86902626
--- Diff: src/java/main/org/apache/zookeeper/cli/DeleteCommand.java ---
@@ -59,11 +59,6 @@ private void retainCompatibility(String[] cmdArgs) throws CliParseException {
// delete path [version]
if (args.length > 2) {
// rewrite to option
--- End diff --
you can remove the comment too :)
---
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.
---