RFR(xs): 8150379: [windows] Fix Leaks in perfMemory_windows.cpp

[Thomas Stüfe]

Hi David,

thanks for the review!

new webrev:
http://cr.openjdk.java.net/~stuefe/webrevs/8150379-fix-leaks-perfMemory_windows_cpp/webrev.01/webrev/

comments inline.

On Tue, Feb 23, 2016 at 2:24 AM, David Holmes <david.holmes at oracle.com>
wrote:

> Hi Thomas,
>
> On 23/02/2016 12:43 AM, Thomas Stüfe wrote:
>
>> Hi all,
>>
>> please take a look at this small fix for two leaks
>> in perfMemory_windows.cpp.
>>
>> bug report: https://bugs.openjdk.java.net/browse/JDK-8150379
>>
>> webrev:
>>
>> http://cr.openjdk.java.net/~stuefe/webrevs/8150379-fix-leaks-perfMemory_windows_cpp/webrev.00/webrev/
>>
>
> Fixes look good, but there seems to be additional leakage here:
>
> 1437   // get the name of the user associated with this process
> 1438   char* user = get_user_name();
> 1439
> 1440   if (user == NULL) {
> 1441     return NULL;
> 1442   }
> 1443
> 1444   // construct the name of the user specific temporary directory
> 1445   char* dirname = get_user_tmp_dir(user);
> 1446
> 1447   // check that the file system is secure - i.e. it supports ACLs.
> 1448   if (!is_filesystem_secure(dirname)) {
> 1449     return NULL;
> 1450   }
>
> We need to free user and dirname before returning. These allocation
> methods are crying out for some RIIA assistance. :(
>
>
I fixed this place too and took a look at other functions returning C heap
array but did not find anything more. I also played around with an RAII
object, but the change got big quickly, so I decided to keep the change
small. I agree this is a mess. If we wanted to clean up this coding, I
would do this as a separate issue. Also, instead of doing a RAII wrapper,
could we not just move some allocations to resource area?

Thomas

Thanks,
> David
>
> Thank you,
>>
>> Thomas
>>
>>