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

[Endi Sukma Dewata]

On 6/19/2012 3:20 PM, Abhishek Koneru wrote:
> Please review the attached patch which has the fixes for Resource leak
> cases in Coverity, for DogTag 10.

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