Merge stable-2.x into master #107
Conversation
…port are null or empty (jenkinsci#101) Bug diagnosis: * https://scan8.coverity.com/reports.htm#v14462/p10499/fileInstanceId=12836372&defectInstanceId=4427367&mergedDefectId=152195 * https://scan8.coverity.com/reports.htm#v14462/p10499/fileInstanceId=12836372&defectInstanceId=4427365&mergedDefectId=152194
…ptional paths (jenkinsci#104) * [CID-152200,CID-152202] - Resource leak in Cipher I/O streams on exceptional paths * https://scan8.coverity.com/reports.htm#v14462/p10499/fileInstanceId=12836377&defectInstanceId=4427377&mergedDefectId=152202 * https://scan8.coverity.com/reports.htm#v14462/p10499/fileInstanceId=12836377&defectInstanceId=4427377&mergedDefectId=152200 * [CID-152202] - Fix typo noticed by @olivergondza
….x to master Conflicts: pom.xml src/main/java/hudson/remoting/Engine.java src/main/java/hudson/remoting/Util.java src/main/java/org/jenkinsci/remoting/engine/EngineUtil.java src/main/java/org/jenkinsci/remoting/engine/JnlpServer3Handshake.java
| int read = fis.read(cert); | ||
| FileInputStream fis = new FileInputStream(file); | ||
| final int read; | ||
| try { |
There was a problem hiding this comment.
Can be reworked later since we're on Java 7 now
There was a problem hiding this comment.
Reworked using try with reseources?
|
@reviewbybees @stephenc |
|
This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation. |
| * @param closeableName Name of the closeable item | ||
| * @param closeableOwner String representation of the closeable holder | ||
| */ | ||
| static void closeAndLogFailures(@CheckForNull Closeable toClose, @Nonnull Logger logger, |
There was a problem hiding this comment.
Seems "no", the merge process removed the usage. But it will be definitely used when I start cleaning up FindBugs
There was a problem hiding this comment.
So I want to keep it if there is no strong -1s
There was a problem hiding this comment.
🐜; this code idiom was never a good idea in Java 6- IMO (better to just let any hypothetical exception from close be thrown up the stack), and is completely unnecessary in Java 7+, since try-with-resources calls addSuppressed when necessary. So delete this unused method, and when fixing FB warnings, just use the modern try style and you are fine.
| * @param closeableName Name of the closeable item | ||
| * @param closeableOwner String representation of the closeable holder | ||
| */ | ||
| static void closeAndLogFailures(@CheckForNull Closeable toClose, @Nonnull Logger logger, |
There was a problem hiding this comment.
🐜; this code idiom was never a good idea in Java 6- IMO (better to just let any hypothetical exception from close be thrown up the stack), and is completely unnecessary in Java 7+, since try-with-resources calls addSuppressed when necessary. So delete this unused method, and when fixing FB warnings, just use the modern try style and you are fine.
|
The method is still useful for particular cases, but I'll remove it in order to unblock the PR and further changes |
|
Merging as per previous feedback |
Integrates pending changes from the stable branch