Re: RFR : [XS] 8228578: fix CFData object leak in macosx KeystoreImpl.m

[Weijun Wang]

This looks fine to me.

You might want to add a noreg-* label to the bug. Maybe noreg-cleanup?

Thanks,
Max

> On Jul 24, 2019, at 7:35 PM, Baesken, Matthias <matthias.baes...@sap.com> 
> wrote:
> 
> Hello,  here is the webrev for easier review .
>  
> Bug/webrev  (after my webrev creation works again) :
>  
>  
> http://cr.openjdk.java.net/~mbaesken/webrevs/8228578.0/
>  
> https://bugs.openjdk.java.net/browse/JDK-8228578
>  
> Best regards, Matthias
>  
> From: Baesken, Matthias 
> Sent: Mittwoch, 24. Juli 2019 13:17
> To: security-dev@openjdk.java.net
> Cc: 'naoto.s...@oracle.com' <naoto.s...@oracle.com>
> Subject: RFR : [XS] 8228578: fix CFData object leak in macosx KeystoreImpl.m
>  
> Hello,   please  review the  following small  patch .
>  
> In KeystoreImpl.m   we call CFDataCreate at one place.  According to 
> 
> https://developer.apple.com/documentation/corefoundation/1542359-cfdatacreate?language=objc
>  
> 
> the return value of CFDataCreate is :    "A new CFData object, or NULL if 
> there was a problem creating the object. Ownership follows the The Create 
> Rule." 
> 
> Following the "Create Rule" ,  we have to release the return value to avoid 
> leaks. 
> Or do I miss something ?
>  
>  
> Bug /  (no  webrev currently  because  I have some technical issues at the 
> moment with webrev creation, getting connection reset by peer for some reason 
> )
>  
> https://bugs.openjdk.java.net/browse/JDK-8228578
>  
> patch is attached, and change also  below .
>  
> Thanks, Matthias
>  
>  
>  
> Change :
>  
> # HG changeset patch
> # Parent 042dfb697624926507649a4a8ad17a5e6730ba04
> 8228578: fix CFData object leak in macosx KeystoreImpl.m
>  
> diff -r 042dfb697624 -r 9f43fea81900 
> src/java.base/macosx/native/libosxsecurity/KeystoreImpl.m
> --- a/src/java.base/macosx/native/libosxsecurity/KeystoreImpl.m           Tue 
> Jul 23 20:03:03 2019 -0700
> +++ b/src/java.base/macosx/native/libosxsecurity/KeystoreImpl.m        Wed 
> Jul 24 12:36:12 2019 +0200
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2011, 2018, Oracle and/or its affiliates. All rights 
> reserved.
> + * Copyright (c) 2011, 2019, Oracle and/or its affiliates. All rights 
> reserved.
>   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>   *
>   * This code is free software; you can redistribute it and/or modify it
> @@ -562,6 +562,9 @@
>  
>      err = SecKeychainItemImport(cfDataToImport, NULL, &dataFormat, NULL,
>                                  0, &paramBlock, defaultKeychain, 
> &createdItems);
> +    if (cfDataToImport != NULL) {
> +        CFRelease(cfDataToImport);
> +    }
>  
>      if (err == noErr) {
>          SecKeychainItemRef anItem = 
> (SecKeychainItemRef)CFArrayGetValueAtIndex(createdItems, 0);