Hello , I checked a number of other CF*Create* / CF*Copy* methods (and corresp. CFRelease calls) we have in java.base and almost all looked good to me . However this one looked bad to me :
https://hg.openjdk.java.net/jdk/jdk/file/aaa83519e723/src/java.base/macosx/native/libosxsecurity/KeystoreImpl.m#l522 CFDataRef cfDataToImport = CFDataCreate(kCFAllocatorDefault, (UInt8 *)rawData, dataSize); .... err = SecKeychainItemImport(cfDataToImport, NULL, &dataFormat, NULL, 0, ¶mBlock, defaultKeychain, &createdItems); Don't we need a CFRelease here too , the docu https://developer.apple.com/documentation/corefoundation/1542359-cfdatacreate?language=objc says the ownership of the returned object follows the Create rule . Do I miss something ? Best regards, Matthias > -----Original Message----- > From: Baesken, Matthias > Sent: Dienstag, 23. Juli 2019 08:43 > To: core-libs-dev@openjdk.java.net; 'naoto.s...@oracle.com' > <naoto.s...@oracle.com> > Subject: Re: java_props_macosx.c : CFLocaleCopyCurrent() needs CFRelease > ? > > Thanks for your input ! > > I opened > > https://bugs.openjdk.java.net/browse/JDK-8228501 > > for this issue, will provide a patch . > > Best regards, Matthias > > > > Date: Mon, 22 Jul 2019 12:56:50 -0700 > > From: naoto.s...@oracle.com > > To: core-libs-dev@openjdk.java.net > > Subject: Re: java_props_macosx.c : CFLocaleCopyCurrent() needs > > CFRelease ? > > Message-ID: <72d41e80-82d2-44fc-dead-3fa6a653d...@oracle.com> > > Content-Type: text/plain; charset=utf-8; format=flowed > > > > Hi Matthias, > > > > Thanks for catching them. Yes, I believe they should be released > > appropriately. > > > > Naoto > > > > On 7/22/19 4:01 AM, Baesken, Matthias wrote: > > > Hello , maybe someone with more OSX dev knowledge could comment > > on this . > > > If I understand it correctly , the OSX Core Foundation Ownership > > > Policy : > > > > > > > > > https://developer.apple.com/library/archive/documentation/CoreFoundatio > > > n/Conceptual/CFMemoryMgmt/Concepts/Ownership.html#//apple_ref/doc > > /uid/20001148-103029 > > > > > > says that "Object-duplication functions that have "Copy" embedded in > the > > name." (like CFLocaleCopyCurrent ) need to > > > relinquish ownership (using > > > CFRelease<https://developer.apple.com/documentation/corefoundation/1 > > 521153-cfrelease>) when you have finished with it. > > > > > > Should we better add then CFRelease to the 2 CFLocaleCopyCurrent > > usages in src/java.base/macosx/native/libjava/java_props_macosx.c > > (coding below) ? > > > Or do I miss something ? > > > > > > Thanks , Matthias > > > > > > > > > > > > --- a/src/java.base/macosx/native/libjava/java_props_macosx.c Fri Jul > 19 > > 10:18:48 2019 +0200 > > > +++ b/src/java.base/macosx/native/libjava/java_props_macosx.c Mon > Jul > > 22 12:47:21 2019 +0200 > > > @@ -91,18 +91,22 @@ > > > if (hyphenPos == NULL || // languageString contains ISO639 > > > only, > > e.g., "en" > > > languageString + langStrLen - hyphenPos == 5) { // > > > ISO639- > > ScriptCode, e.g., "en-Latn" > > > - > > CFStringGetCString(CFLocaleGetIdentifier(CFLocaleCopyCurrent()), > > > - localeString, LOCALEIDLENGTH, > > CFStringGetSystemEncoding()); > > > - char *underscorePos = strrchr(localeString, '_'); > > > - char *region = NULL; > > > + CFLocaleRef cflocale = CFLocaleCopyCurrent(); > > > + if (cflocale != NULL) { > > > + CFStringGetCString(CFLocaleGetIdentifier(cflocale), > > > + localeString, LOCALEIDLENGTH, > > CFStringGetSystemEncoding()); > > > + char *underscorePos = strrchr(localeString, '_'); > > > + char *region = NULL; > > > - if (underscorePos != NULL) { > > > - region = underscorePos + 1; > > > - } > > > + if (underscorePos != NULL) { > > > + region = underscorePos + 1; > > > + } > > > - if (region != NULL) { > > > - strcat(languageString, "-"); > > > - strcat(languageString, region); > > > + if (region != NULL) { > > > + strcat(languageString, "-"); > > > + strcat(languageString, region); > > > + } > > > + CFRelease(cflocale); > > > } > > > } > > > > > > @@ -112,12 +116,18 @@ > > > default: > > > { > > > - if > > (!CFStringGetCString(CFLocaleGetIdentifier(CFLocaleCopyCurrent()), > > > - localeString, LOCALEIDLENGTH, > > CFStringGetSystemEncoding())) { > > > + CFLocaleRef cflocale = CFLocaleCopyCurrent(); > > > + if (cflocale != NULL) { > > > + if (!CFStringGetCString(CFLocaleGetIdentifier(cflocale), > > > + localeString, LOCALEIDLENGTH, > > CFStringGetSystemEncoding())) { > > > + return NULL; > > > + } > > > + > > > + retVal = localeString; > > > + CFRelease(cflocale); > > > + } else { > > > return NULL; > > > } > > > - > > > - retVal = localeString; > > > } > > > break; > > > } > > > > > > > > > >
