Re: [Pki-devel] [PATCH] 14 Fixes for Some Resource Leaks in DogTag 10 shown in Coverity

On 6/19/2012 3:20 PM, Abhishek Koneru wrote: Some comments: 1. In PKCS12Export.java:242 the in.close() should be moved into a finally block. It might not matter much because the program will exit on exception, but we should keep it consistent. 2. Unlike previous cases, in ConfigureCA.BackupPanel() the streams are used sequentially (not simultaneously), so I think the close() methods should be called at the original positions to make sure the data is flushed before it's read by the next stream. To avoid resource leaks you can use separate try-finally block for each stream: FileOutputStream fos = null; try { fos = new FileOutputStream(...); ... } finally { if (fos != null) fos.close(); } BufferedReader br = null; try { br = new BufferedReader(...); ... } finally { if (br != null) br.close(); } FileInputStream fis = null; try { fis = new FileInputStream(...); ... } finally { if (fis != null) fis.close(); } No need to catch the exception in the above try-finally blocks, you can use the catch block at line 942. 3. Same issue in ConfigureDRM.SavePKCS12Panel(). 4. Same issue in ConfigureOCSP.SavePKCS12Panel(). 5. Same issue in ConfigureTKS.SavePKCS12Panel(). 6. In Con2Agent.java:194 the flush() invocations should not be removed to make sure the request is sent before the response is read using stdin1. The flush() invocations in lines 203-204 can be removed because it's going to be closed anyway. 7. In ServerInfo.java:299 the close() invocation is no longer needed. 8. In TestClient.getProperties() the stream should be closed in a finally block in case the load() throws an exception. 9. In UserEnroll.java:305,313,321 remove the auto-generated comment. 10. Similar to #6, in HTTPClient.java:242 the flush() invocations should not be removed. 11. In HTTPClient.java:1013,1089 the null assignment is not necessary because the variable will be come out of scope as soon as it exits the method. -- Endi S. Dewata