Review Request: RT-37788

[Danno Ferrin]

Try #2, after running it through the the Xcode leak analyzer and unit testing (FYI keep your persistent domains lower case).

WebRev: http://cr.openjdk.java.net/~shemnon/RT-37788/webrev.01/rt.patch
JIRA: https://javafx-jira.kenai.com/browse/RT-37788

And let’s move further discussion to the JIRA issue.

On Jul 2, 2014, at 10:22 PM, Petr Pchelko <petr.pchelko at oracle.com> wrote:

> Hello, Danno.
> 
> I’ve noticed that you are leaking userDefaults and expandedOptions objects. 
> Adding autorelease to them would save you some memory.
> 
> With best regards. Petr.
> 
>> On Jul 3, 2014, at 1:43 AM, Danno Ferrin <danno.ferrin at oracle.com> wrote:
>> 
>> Yes, this has to be fixed in native code.  8u40 it is then.
>> 
>> I can make it cause a crash, but that starts with shooting myself in the foot, and not much can be done to fix it (by passing in bogus VM arguments).
>> 
>> 
>> On Jul 2, 2014, at 3:40 PM, Stephen F Northover <steve.x.northover at oracle.com> wrote:
>> 
>>> Personally, I wouldn't change any native code at this point unless it was fixing a crash.  The review is for 8u40, correct?
>>> 
>>> Steve
>>> 
>>> On 2014-07-02, 5:38 PM, Chris Bensen wrote:
>>>> I’m not sure about for 8u20. Seems fairly straight forward, and your Obj-C seems as good as any Obj-C. My only complaint at the moment is the following:
>>>> 358     if ([pathParts count] > 2) {
>>>> 359         // for 3 or more steps, the domain is first.second.third and the keys are "/first/second/third/", "fourth/", "fifth/"... etc
>>>> 360         persistentDomain = [NSString stringWithFormat: @"%@.%@.%@", [pathParts objectAtIndex: 0],
>>>> 361                             [pathParts objectAtIndex: 1], [pathParts objectAtIndex: 2]];
>>>> 362         
>>>> 363         [dictPath replaceObjectAtIndex: 0 withObject: [NSString stringWithFormat:@"/%@/%@/%@", [pathParts objectAtIndex: 0],
>>>> 364                                                        [pathParts objectAtIndex: 1], [pathParts objectAtIndex: 2]]];
>>>> 365         [dictPath removeObjectAtIndex: 2];
>>>> 366         [dictPath removeObjectAtIndex: 1];
>>>> 367     } else {
>>>> 368         // for 1 or two steps, the domain is first.second.third and the keys are "/", "first/", "second/"
>>>> 369         persistentDomain = @DEFAULT_JAVA_PREFS_DOMAIN;
>>>> 370         [dictPath insertObject: @"" atIndex:0];
>>>> 371     }
>>>> 
>>>> what if [pathParts count] is 0? I’d probably do a switch:
>>>> 
>>>> switch ([pathParts count]) {
>>>>  case 0:
>>>>     //error
>>>>     return/break;
>>>>  case 1:
>>>>  case 2:
>>>> 368         // for 1 or two steps, the domain is first.second.third and the keys are "/", "first/", "second/"
>>>> 369         persistentDomain = @DEFAULT_JAVA_PREFS_DOMAIN;
>>>> 370         [dictPath insertObject: @"" atIndex:0];
>>>>  default:
>>>> 359         // for 3 or more steps, the domain is first.second.third and the keys are "/first/second/third/", "fourth/", "fifth/"... etc
>>>> 360         persistentDomain = [NSString stringWithFormat: @"%@.%@.%@", [pathParts objectAtIndex: 0],
>>>> 361                             [pathParts objectAtIndex: 1], [pathParts objectAtIndex: 2]];
>>>> 362         
>>>> 363         [dictPath replaceObjectAtIndex: 0 withObject: [NSString stringWithFormat:@"/%@/%@/%@", [pathParts objectAtIndex: 0],
>>>> 364                                                        [pathParts objectAtIndex: 1], [pathParts objectAtIndex: 2]]];
>>>> 365         [dictPath removeObjectAtIndex: 2];
>>>> 366         [dictPath removeObjectAtIndex: 1];
>>>> 
>>>> }
>>>> 
>>>> 
>>>> Make sense? Clear as mud?
>>>> 
>>>> Chris
>>>> 
>>>> 
>>>> On Jul 2, 2014, at 2:15 PM, Danno Ferrin <danno.ferrin at oracle.com> wrote:
>>>> 
>>>>> Chris, Kevin, Steve,
>>>>> 
>>>>> Please review this fix for RT-37788.  Since I am not an objective C any comments are welcome.  Also, please consider if this is too much for an 8u20 fix (the diff is against the current 8u40 codebase).
>>>>> 
>>>>> Webrev: http://cr.openjdk.java.net/~shemnon/RT-37788/webrev.00/
>>>>> JIRA: https://javafx-jira.kenai.com/browse/RT-37788
>>>>> 
>>>>> —Danno
>