I don't think so. --Max
> On Jul 25, 2019, at 5:09 PM, Baesken, Matthias <[email protected]> > wrote: > > Thanks for the review . > Do I need a second review for this one ? > > > Best regards, Matthias > > >> -----Original Message----- >> From: Weijun Wang <[email protected]> >> Sent: Mittwoch, 24. Juli 2019 15:41 >> To: Baesken, Matthias <[email protected]> >> Cc: [email protected]; [email protected] >> Subject: Re: RFR : [XS] 8228578: fix CFData object leak in macosx >> KeystoreImpl.m >> >> 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 >> <[email protected]> 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: [email protected] >>> Cc: '[email protected]' <[email protected]> >>> 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, ¶mBlock, defaultKeychain, >>> &createdItems); >>> + if (cfDataToImport != NULL) { >>> + CFRelease(cfDataToImport); >>> + } >>> >>> if (err == noErr) { >>> SecKeychainItemRef anItem = >> (SecKeychainItemRef)CFArrayGetValueAtIndex(createdItems, 0); >
